[nft PATCH 1/4] evaluate: refactor function to check the reject family in inet and bridge

[Alvaro Neira Ayuso]

This patch make a refactorization of the code to check the reject family in inet
and bridge. These changes will be used in follow up patches.

Signed-off-by: Alvaro Neira Ayuso <***@gmail.com>
---
src/evaluate.c | 110 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 63 insertions(+), 47 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 1fec120..977df86 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1202,12 +1202,72 @@ static int stmt_reject_gen_dependency(struct eval_ctx *ctx, struct stmt *stmt,
return 0;
}

-static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt,
+static int stmt_evaluate_reject_inet(struct eval_ctx *ctx, struct stmt *stmt,
+ struct expr *expr)
+{
+ const struct proto_desc *desc, *base;
+ int protocol;
+
+ base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
+ desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
+ if (desc != NULL) {
+ protocol = proto_find_num(base, desc);
+ switch (protocol) {
+ case NFPROTO_IPV4:
+ if (stmt->reject.family == NFPROTO_IPV4)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ case NFPROTO_IPV6:
+ if (stmt->reject.family == NFPROTO_IPV6)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ default:
+ BUG("unsupported family");
+ }
+ }
+ if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
+ return 0;
+ if (stmt_reject_gen_dependency(ctx, stmt, expr) < 0)
+ return -1;
+ return 0;
+}
+
+static int stmt_evaluate_reject_bridge(struct eval_ctx *ctx, struct stmt *stmt,
struct expr *expr)
{
const struct proto_desc *desc, *base;
int protocol;

+ base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
+ desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
+ if (desc != NULL) {
+ protocol = proto_find_num(base, desc);
+ switch (protocol) {
+ case __constant_htons(ETH_P_IP):
+ if (NFPROTO_IPV4 == stmt->reject.family)
+ return 0;
+ case __constant_htons(ETH_P_IPV6):
+ if (NFPROTO_IPV6 == stmt->reject.family)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ default:
+ return stmt_error(ctx, stmt,
+ "cannot reject this ether type");
+ }
+ }
+ if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
+ return 0;
+ if (stmt_reject_gen_dependency(ctx, stmt, expr) < 0)
+ return -1;
+ return 0;
+}
+
+static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt,
+ struct expr *expr)
+{
switch (ctx->pctx.family) {
case NFPROTO_ARP:
return stmt_error(ctx, stmt, "cannot use reject with arp");
@@ -1229,55 +1289,11 @@ static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt,
}
break;
case NFPROTO_BRIDGE:
- base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
- desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
- if (desc != NULL) {
- protocol = proto_find_num(base, desc);
- switch (protocol) {
- case __constant_htons(ETH_P_IP):
- if (NFPROTO_IPV4 == stmt->reject.family)
- break;
- case __constant_htons(ETH_P_IPV6):
- if (NFPROTO_IPV6 == stmt->reject.family)
- break;
- return stmt_error(ctx, stmt,
- "conflicting protocols specified: ip vs ip6");
- default:
- return stmt_error(ctx, stmt,
- "cannot reject this ether type");
- }
- break;
- }
- if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
- break;
- if (stmt_reject_gen_dependency(ctx, stmt, expr) < 0)
+ if (stmt_evaluate_reject_bridge(ctx, stmt, expr) < 0)
return -1;
break;
case NFPROTO_INET:
- base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
- desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
- if (desc != NULL) {
- protocol = proto_find_num(base, desc);
- switch (protocol) {
- case NFPROTO_IPV4:
- if (stmt->reject.family == NFPROTO_IPV4)
- break;
- return stmt_error(ctx, stmt,
- "conflicting protocols specified: ip vs ip6");
- break;
- case NFPROTO_IPV6:
- if (stmt->reject.family == NFPROTO_IPV6)
- break;
- return stmt_error(ctx, stmt,
- "conflicting protocols specified: ip vs ip6");
- default:
- BUG("unsupported family");
- }
- break;
- }
- if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
- break;
- if (stmt_reject_gen_dependency(ctx, stmt, expr) < 0)
+ if (stmt_evaluate_reject_inet(ctx, stmt, expr) < 0)
return -1;
break;
}

[Alvaro Neira Ayuso]

nft add rule -nnn bridge test-bridge input \
ip protocol tcp reject with tcp reset

If we use in reject the type tcp reset. We don't need to check if the network
context is compatible with the reason. This patch fix that.

Signed-off-by: Alvaro Neira Ayuso <***@gmail.com>
---
src/evaluate.c | 80 +++++++++++++++++++++++++++++++++-----------------------
1 file changed, 47 insertions(+), 33 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 20235a8..8b19baf 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1208,24 +1208,31 @@ static int stmt_evaluate_reject_inet(struct eval_ctx *ctx, struct stmt *stmt,
const struct proto_desc *desc, *base;
int protocol;

- base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
- desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
- if (desc != NULL) {
- protocol = proto_find_num(base, desc);
- switch (protocol) {
- case NFPROTO_IPV4:
- if (stmt->reject.family == NFPROTO_IPV4)
- return 0;
- return stmt_error(ctx, stmt,
- "conflicting protocols specified: ip vs ip6");
- case NFPROTO_IPV6:
- if (stmt->reject.family == NFPROTO_IPV6)
- return 0;
- return stmt_error(ctx, stmt,
- "conflicting protocols specified: ip vs ip6");
- default:
- BUG("unsupported family");
+ switch (stmt->reject.type) {
+ case NFT_REJECT_TCP_RST:
+ break;
+ case NFT_REJECT_ICMPX_UNREACH:
+ case NFT_REJECT_ICMP_UNREACH:
+ base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
+ desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
+ if (desc != NULL) {
+ protocol = proto_find_num(base, desc);
+ switch (protocol) {
+ case NFPROTO_IPV4:
+ if (stmt->reject.family == NFPROTO_IPV4)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ case NFPROTO_IPV6:
+ if (stmt->reject.family == NFPROTO_IPV6)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ default:
+ BUG("unsupported family");
+ }
}
+ break;
}
if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
return 0;
@@ -1240,25 +1247,32 @@ static int stmt_evaluate_reject_bridge(struct eval_ctx *ctx, struct stmt *stmt,
const struct proto_desc *desc, *base;
int protocol;

- base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
- desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
- if (desc != NULL) {
- protocol = proto_find_num(base, desc);
- switch (protocol) {
- case __constant_htons(ETH_P_IP):
- if (NFPROTO_IPV4 == stmt->reject.family)
- return 0;
- return stmt_error(ctx, stmt,
+ switch (stmt->reject.type) {
+ case NFT_REJECT_TCP_RST:
+ break;
+ case NFT_REJECT_ICMPX_UNREACH:
+ case NFT_REJECT_ICMP_UNREACH:
+ base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
+ desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
+ if (desc != NULL) {
+ protocol = proto_find_num(base, desc);
+ switch (protocol) {
+ case __constant_htons(ETH_P_IP):
+ if (NFPROTO_IPV4 == stmt->reject.family)
+ return 0;
+ return stmt_error(ctx, stmt,
"conflicting protocols specified: ip vs ip6");
- case __constant_htons(ETH_P_IPV6):
- if (NFPROTO_IPV6 == stmt->reject.family)
- return 0;
- return stmt_error(ctx, stmt,
+ case __constant_htons(ETH_P_IPV6):
+ if (NFPROTO_IPV6 == stmt->reject.family)
+ return 0;
+ return stmt_error(ctx, stmt,
"conflicting protocols specified: ip vs ip6");
- default:
- return stmt_error(ctx, stmt,
- "cannot reject this ether type");
+ default:
+ return stmt_error(ctx, stmt,
+ "cannot reject this ether type");
+ }
}
+ break;
}
if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
return 0;

[Pablo Neira Ayuso]

Post by Alvaro Neira Ayuso
nft add rule -nnn bridge test-bridge input \
ip protocol tcp reject with tcp reset
If we use in reject the type tcp reset. We don't need to check if the network
context is compatible with the reason. This patch fix that.
---
src/evaluate.c | 80 +++++++++++++++++++++++++++++++++-----------------------
1 file changed, 47 insertions(+), 33 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 20235a8..8b19baf 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1208,24 +1208,31 @@ static int stmt_evaluate_reject_inet(struct eval_ctx *ctx, struct stmt *stmt,
const struct proto_desc *desc, *base;
int protocol;
- base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
- desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
- if (desc != NULL) {
- protocol = proto_find_num(base, desc);
- switch (protocol) {
- if (stmt->reject.family == NFPROTO_IPV4)
- return 0;
- return stmt_error(ctx, stmt,
- "conflicting protocols specified: ip vs ip6");
- if (stmt->reject.family == NFPROTO_IPV6)
- return 0;
- return stmt_error(ctx, stmt,
- "conflicting protocols specified: ip vs ip6");
- BUG("unsupported family");
+ switch (stmt->reject.type) {
+ break;

Do you really need to check layer 3 conflicts with icmpx?

Post by Alvaro Neira Ayuso
+ base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
+ desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
+ if (desc != NULL) {
+ protocol = proto_find_num(base, desc);
+ switch (protocol) {
+ if (stmt->reject.family == NFPROTO_IPV4)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ if (stmt->reject.family == NFPROTO_IPV6)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ BUG("unsupported family");
+ }
}
+ break;
}
if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
return 0;

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Álvaro Neira Ayuso]

Post by Pablo Neira Ayuso

Post by Alvaro Neira Ayuso
nft add rule -nnn bridge test-bridge input \
ip protocol tcp reject with tcp reset
If we use in reject the type tcp reset. We don't need to check if th=

e network

Post by Pablo Neira Ayuso

Post by Alvaro Neira Ayuso
context is compatible with the reason. This patch fix that.
---
src/evaluate.c | 80 +++++++++++++++++++++++++++++++++-----------=

------------

Post by Pablo Neira Ayuso

Post by Alvaro Neira Ayuso
1 file changed, 47 insertions(+), 33 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 20235a8..8b19baf 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1208,24 +1208,31 @@ static int stmt_evaluate_reject_inet(struct =

eval_ctx *ctx, struct stmt *stmt,

Post by Pablo Neira Ayuso

Post by Alvaro Neira Ayuso
const struct proto_desc *desc, *base;
int protocol;
- base =3D ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
- desc =3D ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
- if (desc !=3D NULL) {
- protocol =3D proto_find_num(base, desc);
- switch (protocol) {
- if (stmt->reject.family =3D=3D NFPROTO_IPV4)
- return 0;
- return stmt_error(ctx, stmt,
- "conflicting protocols specified: ip vs ip6");
- if (stmt->reject.family =3D=3D NFPROTO_IPV6)
- return 0;
- return stmt_error(ctx, stmt,
- "conflicting protocols specified: ip vs ip6");
- BUG("unsupported family");
+ switch (stmt->reject.type) {
+ break;

Do you really need to check layer 3 conflicts with icmpx?

Ups you're right. In Inet tables we don't need to check it. We only hav=
e=20
Ipv4 and Ipv6 traffic. I have been focus in bridge and I have followed=20
the same steps (check the network context) like bridge tables and in=20
Inet is not necessary.

I'm going to fix it. Thanks Pablo

Post by Pablo Neira Ayuso

Post by Alvaro Neira Ayuso
+ base =3D ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;
+ desc =3D ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
+ if (desc !=3D NULL) {
+ protocol =3D proto_find_num(base, desc);
+ switch (protocol) {
+ if (stmt->reject.family =3D=3D NFPROTO_IPV4)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ if (stmt->reject.family =3D=3D NFPROTO_IPV6)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ BUG("unsupported family");
+ }
}
+ break;
}
if (stmt->reject.type =3D=3D NFT_REJECT_ICMPX_UNREACH)
return 0;

--
To unsubscribe from this list: send the line "unsubscribe netfilter-dev=
el" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Alvaro Neira Ayuso]

nft add rule bridge test-bridge input ether type ip \
reject with icmpv6 type no-route

This rule pass the evaluation step but the network context is incompatible with
the reject reason. In that cases, we have to throw an error like "conflicting
protocols specified: ip vs ip6"

Signed-off-by: Alvaro Neira Ayuso <***@gmail.com>
---
src/evaluate.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/src/evaluate.c b/src/evaluate.c
index 977df86..20235a8 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1248,6 +1248,8 @@ static int stmt_evaluate_reject_bridge(struct eval_ctx *ctx, struct stmt *stmt,
case __constant_htons(ETH_P_IP):
if (NFPROTO_IPV4 == stmt->reject.family)
return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
case __constant_htons(ETH_P_IPV6):
if (NFPROTO_IPV6 == stmt->reject.family)
return 0;

[Alvaro Neira Ayuso]

Example:

nft add rule inet filter input meta l4proto udp reject with tcp reset

If we try to check if the transport protocol is tcp, we use the network context.
If we don't have this network context, we have a crash.

Signed-off-by: Alvaro Neira Ayuso <***@gmail.com>
---
[changes in v3]
* Use the proto_inet_service proto in cases that we don't have network context

src/evaluate.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/src/evaluate.c b/src/evaluate.c
index 8b19baf..fc3b1a2 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1365,6 +1365,12 @@ static int stmt_evaluate_reset(struct eval_ctx *ctx, struct stmt *stmt)
if (desc == NULL)
return 0;

+ /* If we don't have network context in inet or bridge */
+ if (base == NULL &&
+ (ctx->pctx.family == NFPROTO_INET ||
+ ctx->pctx.family == NFPROTO_BRIDGE))
+ base = &proto_inet_service;
+
protonum = proto_find_num(base, desc);
switch (protonum) {
case IPPROTO_TCP:

[Pablo Neira Ayuso]

Post by Alvaro Neira Ayuso
nft add rule inet filter input meta l4proto udp reject with tcp reset
If we try to check if the transport protocol is tcp, we use the network context.
If we don't have this network context, we have a crash.

I have applied this with minor changes.

Please, select the patch ordering carefully. This is a v3 coming as
the 4th patch in a batch.

Remember: Fixes always come in first place. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Álvaro Neira Ayuso]

Post by Pablo Neira Ayuso

nft add rule inet filter input meta l4proto udp reject with tcp rese=

t

Post by Pablo Neira Ayuso

If we try to check if the transport protocol is tcp, we use the netw=

ork context.

Post by Pablo Neira Ayuso

If we don't have this network context, we have a crash.

I have applied this with minor changes.
Please, select the patch ordering carefully. This is a v3 coming as
the 4th patch in a batch.
Remember: Fixes always come in first place. Thanks.

Catched. Sorry.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-dev=
el" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Pablo Neira Ayuso]

Post by Alvaro Neira Ayuso
This patch make a refactorization of the code to check the reject family in inet
and bridge. These changes will be used in follow up patches.
---
src/evaluate.c | 110 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 63 insertions(+), 47 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 1fec120..977df86 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1202,12 +1202,72 @@ static int stmt_reject_gen_dependency(struct eval_ctx *ctx, struct stmt *stmt,
return 0;
}
-static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt,
+static int stmt_evaluate_reject_inet(struct eval_ctx *ctx, struct stmt *stmt,
+ struct expr *expr)
+{
+ const struct proto_desc *desc, *base;
+ int protocol;
+
+ base = ctx->pctx.protocol[PROTO_BASE_LL_HDR].desc;

This base pointer is fetched, but you only need this if desc != NULL.
Please, while you're refactoring this, it's good if you avoid this.

Post by Alvaro Neira Ayuso
+ desc = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
+ if (desc != NULL) {
+ protocol = proto_find_num(base, desc);
+ switch (protocol) {
+ if (stmt->reject.family == NFPROTO_IPV4)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ if (stmt->reject.family == NFPROTO_IPV6)
+ return 0;
+ return stmt_error(ctx, stmt,
+ "conflicting protocols specified: ip vs ip6");
+ BUG("unsupported family");
+ }
+ }

to there. You can put this code in another function, given that you'll
need it again for your patch 3/4.

static int stmt_evaluate_reject_inet_family(...)
{
int protocol;

protocol = proto_find_num(base, desc);
switch (protocol) {
case NFPROTO_IPV4:
if (stmt->reject.family != NFPROTO_IPV4)
return -1;

break;
case NFPROTO_IPV6:
if (stmt->reject.family == NFPROTO_IPV6)
return -1;

break;
default:
BUG("unsupported family");
}

return 0;
}

Then, from stmt_evaluate_reject_inet():

if (desc != NULL &&
stmt_evaluate_reject_inet_family(...) < 0)
return stmt_error(..., "conflicting protocols...");

Post by Alvaro Neira Ayuso
+ if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
+ return 0;
+ if (stmt_reject_gen_dependency(ctx, stmt, expr) < 0)
+ return -1;
+ return 0;
+}
+
+static int stmt_evaluate_reject_bridge(struct eval_ctx *ctx, struct stmt *stmt,
struct expr *expr)

Same thing for the bridge code.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html