From 1258053e29d80addcc11865a921c9bfaa91eb903 Mon Sep 17 00:00:00 2001 From: xinghe Date: Sat, 3 Sep 2022 16:26:50 +0800 Subject: [PATCH] fix Community bugs --- ...-cache-validate-handle-string-length.patch | 157 ++++++++++++++++++ ...-when-adding-elements-to-invalid-set.patch | 63 +++++++ ...-fix-device-parsing-in-netdev-family.patch | 61 +++++++ ...ee-add-string-range-reversal-support.patch | 100 +++++++++++ ...-map-listing-with-interface-wildcard.patch | 46 +++++ ...-range-creation-to-a-helper-function.patch | 143 ++++++++++++++++ ...Don-t-parse-string-as-verdict-in-map.patch | 120 +++++++++++++ nftables.spec | 22 ++- 8 files changed, 711 insertions(+), 1 deletion(-) create mode 100644 backport-cache-validate-handle-string-length.patch create mode 100644 backport-evaluate-fix-segfault-when-adding-elements-to-invalid-set.patch create mode 100644 backport-parser_json-fix-device-parsing-in-netdev-family.patch create mode 100644 backport-segtree-add-string-range-reversal-support.patch create mode 100644 backport-segtree-fix-map-listing-with-interface-wildcard.patch create mode 100644 backport-segtree-split-prefix-and-range-creation-to-a-helper-function.patch create mode 100644 backport-src-Don-t-parse-string-as-verdict-in-map.patch diff --git a/backport-cache-validate-handle-string-length.patch b/backport-cache-validate-handle-string-length.patch new file mode 100644 index 0000000..90b05fe --- /dev/null +++ b/backport-cache-validate-handle-string-length.patch @@ -0,0 +1,157 @@ +From 649b8ce38fc8d90127e8c3d5d62a494a86fb1a01 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +Date: Mon, 18 Jul 2022 16:18:33 +0200 +Subject: [PATCH] cache: validate handle string length + +Maximum supported string length for handle is NFT_NAME_MAXLEN, report an +error if user is exceeding this limit. + +By validating from the cache evaluation phase, input is validated for the +native and json parsers. + +Conflict: add struct list_head *msgs; +Reference: https://git.netfilter.org/nftables/commit?id=649b8ce38fc8d90127e8c3d5d62a494a86fb1a01 + +Signed-off-by: Pablo Neira Ayuso +--- + src/cache.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 108 insertions(+) + +diff --git a/src/cache.c b/src/cache.c +index c1f0972..828e4cc 100644 +--- a/src/cache.c ++++ b/src/cache.c +@@ -16,6 +16,7 @@ + #include + #include + #include ++#include + + static unsigned int evaluate_cache_add(struct cmd *cmd, unsigned int flags) + { +@@ -58,6 +59,109 @@ static unsigned int evaluate_cache_add(struct cmd *cmd, unsigned int flags) + return flags; + } + ++static int nft_handle_validate(const struct cmd *cmd, struct list_head *msgs) ++{ ++ const struct handle *h = &cmd->handle; ++ const struct location *loc; ++ ++ switch (cmd->obj) { ++ case CMD_OBJ_TABLE: ++ if (h->table.name && ++ strlen(h->table.name) > NFT_NAME_MAXLEN) { ++ loc = &h->table.location; ++ goto err_name_too_long; ++ } ++ break; ++ case CMD_OBJ_RULE: ++ case CMD_OBJ_CHAIN: ++ case CMD_OBJ_CHAINS: ++ if (h->table.name && ++ strlen(h->table.name) > NFT_NAME_MAXLEN) { ++ loc = &h->table.location; ++ goto err_name_too_long; ++ } ++ if (h->chain.name && ++ strlen(h->chain.name) > NFT_NAME_MAXLEN) { ++ loc = &h->chain.location; ++ goto err_name_too_long; ++ } ++ break; ++ case CMD_OBJ_ELEMENTS: ++ case CMD_OBJ_SET: ++ case CMD_OBJ_SETS: ++ case CMD_OBJ_MAP: ++ case CMD_OBJ_MAPS: ++ case CMD_OBJ_METER: ++ case CMD_OBJ_METERS: ++ if (h->table.name && ++ strlen(h->table.name) > NFT_NAME_MAXLEN) { ++ loc = &h->table.location; ++ goto err_name_too_long; ++ } ++ if (h->set.name && ++ strlen(h->set.name) > NFT_NAME_MAXLEN) { ++ loc = &h->set.location; ++ goto err_name_too_long; ++ } ++ break; ++ case CMD_OBJ_FLOWTABLE: ++ case CMD_OBJ_FLOWTABLES: ++ if (h->table.name && ++ strlen(h->table.name) > NFT_NAME_MAXLEN) { ++ loc = &h->table.location; ++ goto err_name_too_long; ++ } ++ if (h->flowtable.name && ++ strlen(h->flowtable.name) > NFT_NAME_MAXLEN) { ++ loc = &h->flowtable.location; ++ goto err_name_too_long; ++ } ++ break; ++ case CMD_OBJ_INVALID: ++ case CMD_OBJ_EXPR: ++ case CMD_OBJ_RULESET: ++ case CMD_OBJ_MARKUP: ++ case CMD_OBJ_MONITOR: ++ case CMD_OBJ_SETELEMS: ++ case CMD_OBJ_HOOKS: ++ break; ++ case CMD_OBJ_COUNTER: ++ case CMD_OBJ_COUNTERS: ++ case CMD_OBJ_QUOTA: ++ case CMD_OBJ_QUOTAS: ++ case CMD_OBJ_LIMIT: ++ case CMD_OBJ_LIMITS: ++ case CMD_OBJ_SECMARK: ++ case CMD_OBJ_SECMARKS: ++ case CMD_OBJ_SYNPROXY: ++ case CMD_OBJ_SYNPROXYS: ++ case CMD_OBJ_CT_HELPER: ++ case CMD_OBJ_CT_HELPERS: ++ case CMD_OBJ_CT_TIMEOUT: ++ case CMD_OBJ_CT_EXPECT: ++ if (h->table.name && ++ strlen(h->table.name) > NFT_NAME_MAXLEN) { ++ loc = &h->table.location; ++ goto err_name_too_long; ++ } ++ if (h->obj.name && ++ strlen(h->obj.name) > NFT_NAME_MAXLEN) { ++ loc = &h->obj.location; ++ goto err_name_too_long; ++ } ++ break; ++ } ++ ++ return 0; ++ ++err_name_too_long: ++ erec_queue(error(loc, "name too long, %d characters maximum allowed", ++ NFT_NAME_MAXLEN), ++ msgs); ++ return -1; ++} ++ ++ + static unsigned int evaluate_cache_del(struct cmd *cmd, unsigned int flags) + { + switch (cmd->obj) { +@@ -121,8 +225,12 @@ unsigned int nft_cache_evaluate(struct nft_ctx *nft, struct list_head *cmds) + { + unsigned int flags = NFT_CACHE_EMPTY; + struct cmd *cmd; ++ struct list_head *msgs; + + list_for_each_entry(cmd, cmds, list) { ++ if (nft_handle_validate(cmd, msgs) < 0) ++ return -1; ++ + switch (cmd->op) { + case CMD_ADD: + case CMD_INSERT: +-- +2.33.0 + diff --git a/backport-evaluate-fix-segfault-when-adding-elements-to-invalid-set.patch b/backport-evaluate-fix-segfault-when-adding-elements-to-invalid-set.patch new file mode 100644 index 0000000..9e51e94 --- /dev/null +++ b/backport-evaluate-fix-segfault-when-adding-elements-to-invalid-set.patch @@ -0,0 +1,63 @@ +From 27107b4932b1bd1a39c21e0d7865f3bf220a3857 Mon Sep 17 00:00:00 2001 +From: Peter Tirsek +Date: Sun, 26 Jun 2022 00:47:07 -0500 +Subject: [PATCH] evaluate: fix segfault when adding elements to invalid set + +Adding elements to a set or map with an invalid definition causes nft to +segfault. The following nftables.conf triggers the crash: + + flush ruleset + create table inet filter + set inet filter foo {} + add element inet filter foo { foobar } + +Simply parsing and checking the config will trigger it: + + $ nft -c -f nftables.conf.crash + Segmentation fault + +The error in the set/map definition is correctly caught and queued, but +because the set is invalid and does not contain a key type, adding to it +causes a NULL pointer dereference of set->key within setelem_evaluate(). + +I don't think it's necessary to queue another error since the underlying +problem is correctly detected and reported when parsing the definition +of the set. Simply checking the validity of set->key before using it +seems to fix it, causing the error in the definition of the set to be +reported properly. The element type error isn't caught, but that seems +reasonable since the key type is invalid or unknown anyway: + + $ ./nft -c -f ~/nftables.conf.crash + /home/pti/nftables.conf.crash:3:21-21: Error: set definition does not specify key + set inet filter foo {} + ^ + +[ Add tests to cover this case --pablo ] + +Conflict: remove tests/shell/testcases/sets/errors_0 +Reference: https://git.netfilter.org/nftables/commit?id=27107b4932b1bd1a39c21e0d7865f3bf220a3857 + +Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1597 +Signed-off-by: Peter Tirsek +Signed-off-by: Pablo Neira Ayuso +--- + src/evaluate.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/src/evaluate.c b/src/evaluate.c +index 8de8351..a7c2722 100644 +--- a/src/evaluate.c ++++ b/src/evaluate.c +@@ -3853,6 +3853,9 @@ static int setelem_evaluate(struct eval_ctx *ctx, struct cmd *cmd) + return set_not_found(ctx, &ctx->cmd->handle.set.location, + ctx->cmd->handle.set.name); + ++ if (set->key == NULL) ++ return -1; ++ + ctx->set = set; + expr_set_context(&ctx->ectx, set->key->dtype, set->key->len); + if (expr_evaluate(ctx, &cmd->expr) < 0) +-- +2.33.0 + diff --git a/backport-parser_json-fix-device-parsing-in-netdev-family.patch b/backport-parser_json-fix-device-parsing-in-netdev-family.patch new file mode 100644 index 0000000..b891355 --- /dev/null +++ b/backport-parser_json-fix-device-parsing-in-netdev-family.patch @@ -0,0 +1,61 @@ +From 8efab5527cbcb15cd9bff462b7549c0d6181c003 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +Date: Mon, 1 Aug 2022 16:15:08 +0200 +Subject: [PATCH] parser_json: fix device parsing in netdev family + +json_unpack() function is not designed to take a pre-allocated buffer. + +Conflict: NA +Reference: https://git.netfilter.org/nftables/commit?id=8efab5527cbcb15cd9bff462b7549c0d6181c003 + +Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1612 +Fixes: 3fdc7541fba0 ("src: add multidevice support for netdev chain") +Signed-off-by: Pablo Neira Ayuso +--- + src/parser_json.c | 3 +-- + tests/shell/testcases/json/netdev | 19 +++++++++++++++++++ + 2 files changed, 20 insertions(+), 2 deletions(-) + create mode 100755 tests/shell/testcases/json/netdev + +diff --git a/src/parser_json.c b/src/parser_json.c +index 666aa2f..d434839 100644 +--- a/src/parser_json.c ++++ b/src/parser_json.c +@@ -2746,8 +2746,7 @@ static struct cmd *json_parse_cmd_add_chain(struct json_ctx *ctx, json_t *root, + struct handle h = { + .table.location = *int_loc, + }; +- const char *family = "", *policy = "", *type, *hookstr; +- const char name[IFNAMSIZ]; ++ const char *family = "", *policy = "", *type, *hookstr, *name; + struct chain *chain; + int prio; + +diff --git a/tests/shell/testcases/json/netdev b/tests/shell/testcases/json/netdev +new file mode 100755 +index 0000000..a16a4f5 +--- /dev/null ++++ b/tests/shell/testcases/json/netdev +@@ -0,0 +1,19 @@ ++#!/bin/bash ++ ++ip link add d0 type dummy || { ++ echo "Skipping, no dummy interface available" ++ exit 0 ++} ++trap "ip link del d0" EXIT ++ ++set -e ++ ++$NFT flush ruleset ++$NFT add table inet test ++$NFT add chain inet test c ++ ++$NFT flush ruleset ++ ++RULESET='{"nftables":[{"flush":{"ruleset":null}},{"add":{"table":{"family":"netdev","name":"test_table"}}},{"add":{"chain":{"family":"netdev","table":"test_table","name":"test_chain","type":"filter","hook":"ingress","prio":0,"dev":"d0","policy":"accept"}}}]}' ++ ++$NFT -j -f - <<< $RULESET +-- +2.33.0 + diff --git a/backport-segtree-add-string-range-reversal-support.patch b/backport-segtree-add-string-range-reversal-support.patch new file mode 100644 index 0000000..63c81ff --- /dev/null +++ b/backport-segtree-add-string-range-reversal-support.patch @@ -0,0 +1,100 @@ +From 5e393ea1fc0ad6b59e90103bf83e93b2449d519e Mon Sep 17 00:00:00 2001 +From: Florian Westphal +Date: Sat, 9 Apr 2022 15:58:29 +0200 +Subject: segtree: add string "range" reversal support + +Previous commits allows to use set key as a range, i.e. + + key ifname + flags interval + elements = { eth* } + +and then have it match on any interface starting with 'eth'. + +Listing is broken however, we need to reverse-translate the (128bit) +number back to a string. + +'eth*' is stored as interval +00687465 0000000 .. 00697465 0000000, i.e. "eth-eti", +this adds the needed endianess fixups. + +Conflict: NA +Reference: https://git.netfilter.org/nftables/patch/?id=5e393ea1fc0ad6b59e90103bf83e93b2449d519e + +Signed-off-by: Florian Westphal +Signed-off-by: Pablo Neira Ayuso +--- + src/segtree.c | 47 +++++++++++++++++++++++++++++++++++++++++------ + 1 file changed, 41 insertions(+), 6 deletions(-) + +diff --git a/src/segtree.c b/src/segtree.c +index b4e76bf5..bed8bbcf 100644 +--- a/src/segtree.c ++++ b/src/segtree.c +@@ -1032,6 +1032,33 @@ static struct expr *interval_to_prefix(struct expr *low, struct expr *i, const m + return __expr_to_set_elem(low, prefix); + } + ++static struct expr *interval_to_string(struct expr *low, struct expr *i, const mpz_t range) ++{ ++ unsigned int len = div_round_up(i->len, BITS_PER_BYTE); ++ unsigned int prefix_len, str_len; ++ char data[len + 2]; ++ struct expr *expr; ++ ++ prefix_len = expr_value(i)->len - mpz_scan0(range, 0); ++ ++ if (prefix_len > i->len || prefix_len % BITS_PER_BYTE) ++ return interval_to_prefix(low, i, range); ++ ++ mpz_export_data(data, expr_value(low)->value, BYTEORDER_BIG_ENDIAN, len); ++ ++ str_len = strnlen(data, len); ++ if (str_len >= len || str_len == 0) ++ return interval_to_prefix(low, i, range); ++ ++ data[str_len] = '*'; ++ ++ expr = constant_expr_alloc(&low->location, low->dtype, ++ BYTEORDER_HOST_ENDIAN, ++ (str_len + 1) * BITS_PER_BYTE, data); ++ ++ return __expr_to_set_elem(low, expr); ++} ++ + static struct expr *interval_to_range(struct expr *low, struct expr *i, mpz_t range) + { + struct expr *tmp; +@@ -1130,16 +1157,24 @@ void interval_map_decompose(struct expr *set) + + mpz_and(p, expr_value(low)->value, range); + +- if (!mpz_cmp_ui(range, 0)) ++ if (!mpz_cmp_ui(range, 0)) { ++ if (expr_basetype(low)->type == TYPE_STRING) ++ mpz_switch_byteorder(expr_value(low)->value, low->len / BITS_PER_BYTE); ++ + compound_expr_add(set, expr_get(low)); +- else if ((!range_is_prefix(range) || +- !(i->dtype->flags & DTYPE_F_PREFIX)) || +- mpz_cmp_ui(p, 0)) { +- struct expr *expr = interval_to_range(low, i, range); ++ } else if (range_is_prefix(range) && !mpz_cmp_ui(p, 0)) { ++ struct expr *expr; ++ ++ if (i->dtype->flags & DTYPE_F_PREFIX) ++ expr = interval_to_prefix(low, i, range); ++ else if (expr_basetype(i)->type == TYPE_STRING) ++ expr = interval_to_string(low, i, range); ++ else ++ expr = interval_to_range(low, i, range); + + compound_expr_add(set, expr); + } else { +- struct expr *expr = interval_to_prefix(low, i, range); ++ struct expr *expr = interval_to_range(low, i, range); + + compound_expr_add(set, expr); + } +-- +cgit v1.2.3 diff --git a/backport-segtree-fix-map-listing-with-interface-wildcard.patch b/backport-segtree-fix-map-listing-with-interface-wildcard.patch new file mode 100644 index 0000000..8f6b577 --- /dev/null +++ b/backport-segtree-fix-map-listing-with-interface-wildcard.patch @@ -0,0 +1,46 @@ +From 6c23bfa512187d509ecc188653a6f232b0695d1d Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +Date: Mon, 27 Jun 2022 12:54:23 +0200 +Subject: [PATCH] segtree: fix map listing with interface wildcard + + # nft -f - <<'EOF' + table inet filter { + chain INPUT { + iifname vmap { + "eth0" : jump input_lan, + "wg*" : jump input_vpn + } + } + chain input_lan {} + chain input_vpn {} + } + EOF + # nft list ruleset + nft: segtree.c:578: interval_map_decompose: Assertion `low->len / 8 > 0' failed. + +Conflict: remove tests/shell/testcases/sets/dumps/sets_with_ifnames.nft +Reference: https://git.netfilter.org/nftables/commit?id=6c23bfa512187d509ecc188653a6f232b0695d1d + +Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1617 +Fixes: 5e393ea1fc0a ("segtree: add string "range" reversal support") +Signed-off-by: Pablo Neira Ayuso +--- + src/segtree.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/segtree.c b/src/segtree.c +index 0135a07..cc60879 100644 +--- a/src/segtree.c ++++ b/src/segtree.c +@@ -1160,7 +1160,7 @@ void interval_map_decompose(struct expr *set) + + if (!mpz_cmp_ui(range, 0)) { + if (expr_basetype(low)->type == TYPE_STRING) +- mpz_switch_byteorder(expr_value(low)->value, low->len / BITS_PER_BYTE); ++ mpz_switch_byteorder(expr_value(low)->value, expr_value(low)->len / BITS_PER_BYTE); + + compound_expr_add(set, expr_get(low)); + } else if (range_is_prefix(range) && !mpz_cmp_ui(p, 0)) { +-- +2.33.0 + diff --git a/backport-segtree-split-prefix-and-range-creation-to-a-helper-function.patch b/backport-segtree-split-prefix-and-range-creation-to-a-helper-function.patch new file mode 100644 index 0000000..e948bdb --- /dev/null +++ b/backport-segtree-split-prefix-and-range-creation-to-a-helper-function.patch @@ -0,0 +1,143 @@ +From ada50f84bf5a1475549f3f372834812e7cd8d675 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +Date: Sat, 9 Apr 2022 15:58:26 +0200 +Subject: segtree: split prefix and range creation to a helper function + +No functional change intended. + +Conflict: NA +Reference: https://git.netfilter.org/nftables/patch/src/segtree.c?id=ada50f84bf5a1475549f3f372834812e7cd8d675 + +Signed-off-by: Florian Westphal +Signed-off-by: Pablo Neira Ayuso +--- + src/segtree.c | 95 ++++++++++++++++++++++++++++++++--------------------------- + 1 file changed, 52 insertions(+), 43 deletions(-) + +(limited to 'src/segtree.c') + +diff --git a/src/segtree.c b/src/segtree.c +index a61ea3d2..188cafed 100644 +--- a/src/segtree.c ++++ b/src/segtree.c +@@ -773,6 +773,22 @@ out: + return range; + } + ++static struct expr *__expr_to_set_elem(struct expr *low, struct expr *expr) ++{ ++ struct expr *elem = set_elem_expr_alloc(&low->location, expr); ++ ++ if (low->etype == EXPR_MAPPING) { ++ interval_expr_copy(elem, low->left); ++ ++ elem = mapping_expr_alloc(&low->location, elem, ++ expr_clone(low->right)); ++ } else { ++ interval_expr_copy(elem, low); ++ } ++ ++ return elem; ++} ++ + int get_set_decompose(struct set *cache_set, struct set *set) + { + struct expr *i, *next, *range; +@@ -980,6 +996,38 @@ next: + } + } + ++static struct expr *interval_to_prefix(struct expr *low, struct expr *i, const mpz_t range) ++{ ++ unsigned int prefix_len; ++ struct expr *prefix; ++ ++ prefix_len = expr_value(i)->len - mpz_scan0(range, 0); ++ prefix = prefix_expr_alloc(&low->location, ++ expr_clone(expr_value(low)), ++ prefix_len); ++ prefix->len = expr_value(i)->len; ++ ++ return __expr_to_set_elem(low, prefix); ++} ++ ++static struct expr *interval_to_range(struct expr *low, struct expr *i, mpz_t range) ++{ ++ struct expr *tmp; ++ ++ tmp = constant_expr_alloc(&low->location, low->dtype, ++ low->byteorder, expr_value(low)->len, ++ NULL); ++ ++ mpz_add(range, range, expr_value(low)->value); ++ mpz_set(tmp->value, range); ++ ++ tmp = range_expr_alloc(&low->location, ++ expr_clone(expr_value(low)), ++ tmp); ++ ++ return __expr_to_set_elem(low, tmp); ++} ++ + void interval_map_decompose(struct expr *set) + { + struct expr *i, *next, *low = NULL, *end, *catchall = NULL, *key; +@@ -1065,52 +1113,13 @@ void interval_map_decompose(struct expr *set) + else if ((!range_is_prefix(range) || + !(i->dtype->flags & DTYPE_F_PREFIX)) || + mpz_cmp_ui(p, 0)) { +- struct expr *tmp; +- +- tmp = constant_expr_alloc(&low->location, low->dtype, +- low->byteorder, expr_value(low)->len, +- NULL); +- +- mpz_add(range, range, expr_value(low)->value); +- mpz_set(tmp->value, range); ++ struct expr *expr = interval_to_range(low, i, range); + +- tmp = range_expr_alloc(&low->location, +- expr_clone(expr_value(low)), +- tmp); +- tmp = set_elem_expr_alloc(&low->location, tmp); +- +- if (low->etype == EXPR_MAPPING) { +- interval_expr_copy(tmp, low->left); +- +- tmp = mapping_expr_alloc(&tmp->location, tmp, +- expr_clone(low->right)); +- } else { +- interval_expr_copy(tmp, low); +- } +- +- compound_expr_add(set, tmp); ++ compound_expr_add(set, expr); + } else { +- struct expr *prefix; +- unsigned int prefix_len; +- +- prefix_len = expr_value(i)->len - mpz_scan0(range, 0); +- prefix = prefix_expr_alloc(&low->location, +- expr_clone(expr_value(low)), +- prefix_len); +- prefix->len = expr_value(i)->len; +- +- prefix = set_elem_expr_alloc(&low->location, prefix); +- +- if (low->etype == EXPR_MAPPING) { +- interval_expr_copy(prefix, low->left); +- +- prefix = mapping_expr_alloc(&low->location, prefix, +- expr_clone(low->right)); +- } else { +- interval_expr_copy(prefix, low); +- } ++ struct expr *expr = interval_to_prefix(low, i, range); + +- compound_expr_add(set, prefix); ++ compound_expr_add(set, expr); + } + + if (i->flags & EXPR_F_INTERVAL_END) { +-- +cgit v1.2.3 diff --git a/backport-src-Don-t-parse-string-as-verdict-in-map.patch b/backport-src-Don-t-parse-string-as-verdict-in-map.patch new file mode 100644 index 0000000..c8264ee --- /dev/null +++ b/backport-src-Don-t-parse-string-as-verdict-in-map.patch @@ -0,0 +1,120 @@ +From 9a20f17a7a82ce5ba47047e6c3d2fc921cc1087d Mon Sep 17 00:00:00 2001 +From: Xiao Liang +Date: Fri, 19 Aug 2022 10:40:23 +0800 +Subject: [PATCH] src: Don't parse string as verdict in map + +In verdict map, string values are accidentally treated as verdicts. + +For example: + +table t { + map foo { + type ipv4_addr : verdict + elements = { + 192.168.0.1 : bar + } + } + chain output { + type filter hook output priority mangle; + ip daddr vmap @foo + } +} + +Though "bar" is not a valid verdict (should be "jump bar" or something), +the string is taken as the element value. Then NFTA_DATA_VALUE is sent +to the kernel instead of NFTA_DATA_VERDICT. This would be rejected by +recent kernels. On older ones (e.g. v5.4.x) that don't validate the +type, a warning can be seen when the rule is hit, because of the +corrupted verdict value: + +[5120263.467627] WARNING: CPU: 12 PID: 303303 at net/netfilter/nf_tables_core.c:229 nft_do_chain+0x394/0x500 [nf_tables] + +Indeed, we don't parse verdicts during evaluation, but only chain names, +which is of type string rather than verdict. For example, "jump $var" is +a verdict while "$var" is a string. + +Conflict: NA +Reference: https://git.netfilter.org/nftables/commit?id=9a20f17a7a82ce5ba47047e6c3d2fc921cc1087d + +Fixes: c64457cff967 ("src: Allow goto and jump to a variable") +Signed-off-by: Xiao Liang +Signed-off-by: Florian Westphal +--- + src/datatype.c | 12 ----------- + src/evaluate.c | 3 ++- + tests/shell/testcases/nft-f/0031vmap_string_0 | 21 +++++++++++++++++++ + 3 files changed, 23 insertions(+), 13 deletions(-) + create mode 100755 tests/shell/testcases/nft-f/0031vmap_string_0 + +diff --git a/src/datatype.c b/src/datatype.c +index 7267d60..120da6d 100644 +--- a/src/datatype.c ++++ b/src/datatype.c +@@ -321,23 +321,11 @@ static void verdict_type_print(const struct expr *expr, struct output_ctx *octx) + } + } + +-static struct error_record *verdict_type_parse(struct parse_ctx *ctx, +- const struct expr *sym, +- struct expr **res) +-{ +- *res = constant_expr_alloc(&sym->location, &string_type, +- BYTEORDER_HOST_ENDIAN, +- (strlen(sym->identifier) + 1) * BITS_PER_BYTE, +- sym->identifier); +- return NULL; +-} +- + const struct datatype verdict_type = { + .type = TYPE_VERDICT, + .name = "verdict", + .desc = "netfilter verdict", + .print = verdict_type_print, +- .parse = verdict_type_parse, + }; + + static const struct symbol_table nfproto_tbl = { +diff --git a/src/evaluate.c b/src/evaluate.c +index c6332a1..66ba6a4 100644 +--- a/src/evaluate.c ++++ b/src/evaluate.c +@@ -2426,7 +2426,8 @@ static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt) + if (stmt->expr->verdict != NFT_CONTINUE) + stmt->flags |= STMT_F_TERMINAL; + if (stmt->expr->chain != NULL) { +- if (expr_evaluate(ctx, &stmt->expr->chain) < 0) ++ if (stmt_evaluate_arg(ctx, stmt, &string_type, 0, 0, ++ &stmt->expr->chain) < 0) + return -1; + if (stmt->expr->chain->etype != EXPR_VALUE) { + return expr_error(ctx->msgs, stmt->expr->chain, +diff --git a/tests/shell/testcases/nft-f/0031vmap_string_0 b/tests/shell/testcases/nft-f/0031vmap_string_0 +new file mode 100755 +index 0000000..2af846a +--- /dev/null ++++ b/tests/shell/testcases/nft-f/0031vmap_string_0 +@@ -0,0 +1,21 @@ ++#!/bin/bash ++ ++# Tests parse of corrupted verdicts ++ ++set -e ++ ++RULESET=" ++table ip foo { ++ map bar { ++ type ipv4_addr : verdict ++ elements = { ++ 192.168.0.1 : ber ++ } ++ } ++ ++ chain ber { ++ } ++}" ++ ++$NFT -f - <<< "$RULESET" && exit 1 ++exit 0 +-- +2.33.0 + diff --git a/nftables.spec b/nftables.spec index 7277b3d..2ed2121 100644 --- a/nftables.spec +++ b/nftables.spec @@ -1,6 +1,6 @@ Name: nftables Version: 1.0.0 -Release: 2 +Release: 3 Epoch: 1 Summary: A subsystem of the Linux kernel processing network data License: GPLv2 @@ -9,6 +9,14 @@ Source0: http://ftp.netfilter.org/pub/nftables/nftables-%{version}.tar.bz Source1: nftables.service Source2: nftables.conf +Patch0: backport-cache-validate-handle-string-length.patch +Patch1: backport-evaluate-fix-segfault-when-adding-elements-to-invalid-set.patch +Patch2: backport-segtree-split-prefix-and-range-creation-to-a-helper-function.patch +Patch3: backport-segtree-add-string-range-reversal-support.patch +Patch4: backport-segtree-fix-map-listing-with-interface-wildcard.patch +Patch5: backport-src-Don-t-parse-string-as-verdict-in-map.patch +Patch6: backport-parser_json-fix-device-parsing-in-netdev-family.patch + BuildRequires: gcc flex bison libmnl-devel gmp-devel readline-devel libnftnl-devel docbook2X systemd BuildRequires: iptables-devel jansson-devel python3-devel BuildRequires: chrpath @@ -105,6 +113,18 @@ echo "%{_libdir}" > %{buildroot}/etc/ld.so.conf.d/%{name}-%{_arch}.conf %{python3_sitelib}/nftables/ %changelog +* Sat Sep 03 2022 xinghe - 1:1.0.0-3 +- Type:bugfix +- ID:NA +- SUG:NA +- DESC:fix cache prepare nft_cache evaluate to return error + fix cache validate handle string length + add src support for implicit chain bindings + fix cache release pending rules + fix segtree map listing + parser_json fix device parsing in netdev family + fix src Don't parse string as verdict in map + * Mon Aug 1 2022 huangyu - 1:1.0.0-2 - Type:bugfix - ID:NA -- Gitee