a2ea441692
- Install an improved sample config - Fix permissions of osf-related configs - rule: Fix for potential off-by-one in cmd_add_loc() - netlink_delinearize: Fix suspicious calloc() call - netlink: Avoid memleak in error path of netlink_delinearize_obj() - netlink: Avoid memleak in error path of netlink_delinearize_table() - netlink: Avoid memleak in error path of netlink_delinearize_chain() - netlink: Avoid memleak in error path of netlink_delinearize_set() - json: Drop pointless assignment in exthdr_expr_json() - evaluate: Mark fall through case in str2hooknum() - parser_json: Fix for memleak in tcp option error path - parser_bison: Fix for implicit declaration of isalnum - main: fix nft --help output fallout from 719e4427 - tests: add icmp/6 test where dependency should be left alone - payload: check icmp dependency before removing previous icmp expression Resolves: rhbz#1933117, rhbz#1938823, rhbz#1931790, rhbz#1964987, rhbz#1971600
123 lines
3.9 KiB
Diff
123 lines
3.9 KiB
Diff
From 9230899c6d2be8913646ff1a3b560865c330de7b Mon Sep 17 00:00:00 2001
|
|
From: Florian Westphal <fw@strlen.de>
|
|
Date: Mon, 1 Feb 2021 22:08:54 +0100
|
|
Subject: [PATCH] payload: check icmp dependency before removing previous icmp
|
|
expression
|
|
|
|
nft is too greedy when removing icmp dependencies.
|
|
'icmp code 1 type 2' did remove the type when printing.
|
|
|
|
Be more careful and check that the icmp type dependency of the
|
|
candidate expression (earlier icmp payload expression) has the same
|
|
type dependency as the new expression.
|
|
|
|
Reported-by: Eric Garver <eric@garver.life>
|
|
Reported-by: Michael Biebl <biebl@debian.org>
|
|
Tested-by: Eric Garver <eric@garver.life>
|
|
Fixes: d0f3b9eaab8d77e ("payload: auto-remove simple icmp/icmpv6 dependency expressions")
|
|
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
(cherry picked from commit 533565244d88a818d8828ebabd7625e5a8a4c374)
|
|
Signed-off-by: Phil Sutter <psutter@redhat.com>
|
|
---
|
|
src/payload.c | 63 ++++++++++++++++++++++++++++++++++-----------------
|
|
1 file changed, 42 insertions(+), 21 deletions(-)
|
|
|
|
diff --git a/src/payload.c b/src/payload.c
|
|
index 48529bcf5c514..a77ca55005509 100644
|
|
--- a/src/payload.c
|
|
+++ b/src/payload.c
|
|
@@ -627,6 +627,40 @@ void payload_dependency_release(struct payload_dep_ctx *ctx)
|
|
ctx->pdep = NULL;
|
|
}
|
|
|
|
+static uint8_t icmp_dep_to_type(enum icmp_hdr_field_type t)
|
|
+{
|
|
+ switch (t) {
|
|
+ case PROTO_ICMP_ANY:
|
|
+ BUG("Invalid map for simple dependency");
|
|
+ case PROTO_ICMP_ECHO: return ICMP_ECHO;
|
|
+ case PROTO_ICMP6_ECHO: return ICMP6_ECHO_REQUEST;
|
|
+ case PROTO_ICMP_MTU: return ICMP_DEST_UNREACH;
|
|
+ case PROTO_ICMP_ADDRESS: return ICMP_REDIRECT;
|
|
+ case PROTO_ICMP6_MTU: return ICMP6_PACKET_TOO_BIG;
|
|
+ case PROTO_ICMP6_MGMQ: return MLD_LISTENER_QUERY;
|
|
+ case PROTO_ICMP6_PPTR: return ICMP6_PARAM_PROB;
|
|
+ }
|
|
+
|
|
+ BUG("Missing icmp type mapping");
|
|
+}
|
|
+
|
|
+static bool payload_may_dependency_kill_icmp(struct payload_dep_ctx *ctx, struct expr *expr)
|
|
+{
|
|
+ const struct expr *dep = ctx->pdep->expr;
|
|
+ uint8_t icmp_type;
|
|
+
|
|
+ icmp_type = expr->payload.tmpl->icmp_dep;
|
|
+ if (icmp_type == PROTO_ICMP_ANY)
|
|
+ return false;
|
|
+
|
|
+ if (dep->left->payload.desc != expr->payload.desc)
|
|
+ return false;
|
|
+
|
|
+ icmp_type = icmp_dep_to_type(expr->payload.tmpl->icmp_dep);
|
|
+
|
|
+ return ctx->icmp_type == icmp_type;
|
|
+}
|
|
+
|
|
static bool payload_may_dependency_kill(struct payload_dep_ctx *ctx,
|
|
unsigned int family, struct expr *expr)
|
|
{
|
|
@@ -661,6 +695,14 @@ static bool payload_may_dependency_kill(struct payload_dep_ctx *ctx,
|
|
break;
|
|
}
|
|
|
|
+ if (expr->payload.base == PROTO_BASE_TRANSPORT_HDR &&
|
|
+ dep->left->payload.base == PROTO_BASE_TRANSPORT_HDR) {
|
|
+ if (dep->left->payload.desc == &proto_icmp)
|
|
+ return payload_may_dependency_kill_icmp(ctx, expr);
|
|
+ if (dep->left->payload.desc == &proto_icmp6)
|
|
+ return payload_may_dependency_kill_icmp(ctx, expr);
|
|
+ }
|
|
+
|
|
return true;
|
|
}
|
|
|
|
@@ -680,10 +722,6 @@ void payload_dependency_kill(struct payload_dep_ctx *ctx, struct expr *expr,
|
|
if (payload_dependency_exists(ctx, expr->payload.base) &&
|
|
payload_may_dependency_kill(ctx, family, expr))
|
|
payload_dependency_release(ctx);
|
|
- else if (ctx->icmp_type && ctx->pdep) {
|
|
- fprintf(stderr, "Did not kill \n");
|
|
- payload_dependency_release(ctx);
|
|
- }
|
|
}
|
|
|
|
void exthdr_dependency_kill(struct payload_dep_ctx *ctx, struct expr *expr,
|
|
@@ -707,23 +745,6 @@ void exthdr_dependency_kill(struct payload_dep_ctx *ctx, struct expr *expr,
|
|
}
|
|
}
|
|
|
|
-static uint8_t icmp_dep_to_type(enum icmp_hdr_field_type t)
|
|
-{
|
|
- switch (t) {
|
|
- case PROTO_ICMP_ANY:
|
|
- BUG("Invalid map for simple dependency");
|
|
- case PROTO_ICMP_ECHO: return ICMP_ECHO;
|
|
- case PROTO_ICMP6_ECHO: return ICMP6_ECHO_REQUEST;
|
|
- case PROTO_ICMP_MTU: return ICMP_DEST_UNREACH;
|
|
- case PROTO_ICMP_ADDRESS: return ICMP_REDIRECT;
|
|
- case PROTO_ICMP6_MTU: return ICMP6_PACKET_TOO_BIG;
|
|
- case PROTO_ICMP6_MGMQ: return MLD_LISTENER_QUERY;
|
|
- case PROTO_ICMP6_PPTR: return ICMP6_PARAM_PROB;
|
|
- }
|
|
-
|
|
- BUG("Missing icmp type mapping");
|
|
-}
|
|
-
|
|
/**
|
|
* payload_expr_complete - fill in type information of a raw payload expr
|
|
*
|
|
--
|
|
2.31.1
|
|
|