[PATCH nft 0/4] fix list chain x y with anonymous chains

[PATCH nft 0/4] fix list chain x y with anonymous chains

[Pablo Neira Ayuso]

Hi,

Patches 1 to 3 are cache preparation work.

Then, patch 4 fixes listing

 # nft list chain x y
 table ip x {
        chain y {
                jump {
                        accept
                }
        }
 }

This is broken in the nftables 1.0.1 release.

See https://bugzilla.netfilter.org/show_bug.cgi?id=1577

Pablo Neira Ayuso (4):
  src: do not use the nft_cache_filter object from mnl.c
  cache: do not set error code twice
  cache: add helper function to fill up the rule cache
  src: 'nft list chain' prints anonymous chains correctly

 include/cache.h                               |   3 +
 include/mnl.h                                 |   2 +-
 include/netlink.h                             |   1 +
 src/cache.c                                   | 113 ++++++++++++------
 src/mnl.c                                     |  12 +-
 src/netlink_delinearize.c                     |   8 ++
 .../testcases/cache/0010_implicit_chain_0     |  19 +++
 7 files changed, 113 insertions(+), 45 deletions(-)
 create mode 100755 tests/shell/testcases/cache/0010_implicit_chain_0

-- 
2.30.2

[PATCH nft 1/4] src: do not use the nft_cache_filter object from mnl.c

[Pablo Neira Ayuso]

Pass the table and chain strings to mnl_nft_rule_dump() instead.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/mnl.h |  2 +-
 src/cache.c   |  9 ++++++++-
 src/mnl.c     | 12 +++++-------
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/include/mnl.h b/include/mnl.h
index b006192cf7b2..a4abe1ae3242 100644
--- a/include/mnl.h
+++ b/include/mnl.h
@@ -34,7 +34,7 @@ int mnl_nft_rule_del(struct netlink_ctx *ctx, struct cmd *cmd);
 int mnl_nft_rule_replace(struct netlink_ctx *ctx, struct cmd *cmd);
 
 struct nftnl_rule_list *mnl_nft_rule_dump(struct netlink_ctx *ctx, int family,
-					  const struct nft_cache_filter *filter);
+					  const char *table, const char *chain);
 
 int mnl_nft_chain_add(struct netlink_ctx *ctx, struct cmd *cmd,
 		      unsigned int flags);
diff --git a/src/cache.c b/src/cache.c
index 6494e4743f8d..6ca6bbc6645b 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -478,8 +478,15 @@ static int rule_cache_init(struct netlink_ctx *ctx, const struct handle *h,
 			   const struct nft_cache_filter *filter)
 {
 	struct nftnl_rule_list *rule_cache;
+	const char *table;
+	const char *chain;
 
-	rule_cache = mnl_nft_rule_dump(ctx, h->family, filter);
+	if (filter) {
+		table = filter->list.table;
+		chain = filter->list.chain;
+	}
+
+	rule_cache = mnl_nft_rule_dump(ctx, h->family, table, chain);
 	if (rule_cache == NULL) {
 		if (errno == EINTR)
 			return -1;
diff --git a/src/mnl.c b/src/mnl.c
index 5413f8658f9b..6be991a4827c 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -654,7 +654,7 @@ err_free:
 }
 
 struct nftnl_rule_list *mnl_nft_rule_dump(struct netlink_ctx *ctx, int family,
-					  const struct nft_cache_filter *filter)
+					  const char *table, const char *chain)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nftnl_rule_list *nlr_list;
@@ -662,16 +662,14 @@ struct nftnl_rule_list *mnl_nft_rule_dump(struct netlink_ctx *ctx, int family,
 	struct nlmsghdr *nlh;
 	int ret;
 
-	if (filter && filter->list.table) {
+	if (table) {
 		nlr = nftnl_rule_alloc();
 		if (!nlr)
 			memory_allocation_error();
 
-		nftnl_rule_set_str(nlr, NFTNL_RULE_TABLE,
-				   filter->list.table);
-		if (filter->list.chain)
-			nftnl_rule_set_str(nlr, NFTNL_RULE_CHAIN,
-					   filter->list.chain);
+		nftnl_rule_set_str(nlr, NFTNL_RULE_TABLE, table);
+		if (chain)
+			nftnl_rule_set_str(nlr, NFTNL_RULE_CHAIN, chain);
 	}
 
 	nlr_list = nftnl_rule_list_alloc();
-- 
2.30.2

[PATCH nft 2/4] cache: do not set error code twice

[Pablo Neira Ayuso]

The 'ret' variable is already set to a negative value to report an
error, do not set it again to a negative value.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/cache.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 6ca6bbc6645b..0e9e7fe5381d 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -847,10 +847,8 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 	list_for_each_entry(table, &ctx->nft->cache.table_cache.list, cache.list) {
 		if (flags & NFT_CACHE_SET_BIT) {
 			ret = set_cache_init(ctx, table, set_list);
-			if (ret < 0) {
-				ret = -1;
+			if (ret < 0)
 				goto cache_fails;
-			}
 		}
 		if (flags & NFT_CACHE_SETELEM_BIT) {
 			list_for_each_entry(set, &table->set_cache.list, cache.list) {
@@ -862,10 +860,8 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 
 				ret = netlink_list_setelems(ctx, &set->handle,
 							    set);
-				if (ret < 0) {
-					ret = -1;
+				if (ret < 0)
 					goto cache_fails;
-				}
 			}
 		} else if (flags & NFT_CACHE_SETELEM_MAYBE) {
 			list_for_each_entry(set, &table->set_cache.list, cache.list) {
@@ -877,25 +873,19 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 
 				ret = netlink_list_setelems(ctx, &set->handle,
 							    set);
-				if (ret < 0) {
-					ret = -1;
+				if (ret < 0)
 					goto cache_fails;
-				}
 			}
 		}
 		if (flags & NFT_CACHE_CHAIN_BIT) {
 			ret = chain_cache_init(ctx, table, chain_list);
-			if (ret < 0) {
-				ret = -1;
+			if (ret < 0)
 				goto cache_fails;
-			}
 		}
 		if (flags & NFT_CACHE_FLOWTABLE_BIT) {
 			ret = ft_cache_init(ctx, table, ft_list);
-			if (ret < 0) {
-				ret = -1;
+			if (ret < 0)
 				goto cache_fails;
-			}
 		}
 		if (flags & NFT_CACHE_OBJECT_BIT) {
 			obj_list = obj_cache_dump(ctx, table);
@@ -907,10 +897,8 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 
 			nftnl_obj_list_free(obj_list);
 
-			if (ret < 0) {
-				ret = -1;
+			if (ret < 0)
 				goto cache_fails;
-			}
 		}
 
 		if (flags & NFT_CACHE_RULE_BIT) {
@@ -927,10 +915,8 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 
 				list_move_tail(&rule->list, &chain->rules);
 			}
-			if (ret < 0) {
-				ret = -1;
+			if (ret < 0)
 				goto cache_fails;
-			}
 		}
 	}
 
-- 
2.30.2

[PATCH nft 3/4] cache: add helper function to fill up the rule cache

[Pablo Neira Ayuso]

Add a helper function to dump the rules and add them to the
corresponding chain.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/cache.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 0e9e7fe5381d..14957f2de3a9 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -474,7 +474,7 @@ static int list_rule_cb(struct nftnl_rule *nlr, void *data)
 	return 0;
 }
 
-static int rule_cache_init(struct netlink_ctx *ctx, const struct handle *h,
+static int rule_cache_dump(struct netlink_ctx *ctx, const struct handle *h,
 			   const struct nft_cache_filter *filter)
 {
 	struct nftnl_rule_list *rule_cache;
@@ -811,6 +811,29 @@ static int cache_init_tables(struct netlink_ctx *ctx, struct handle *h,
 	return 0;
 }
 
+static int rule_init_cache(struct netlink_ctx *ctx, struct table *table,
+			   const struct nft_cache_filter *filter)
+{
+	struct rule *rule, *nrule;
+	struct chain *chain;
+	int ret;
+
+	ret = rule_cache_dump(ctx, &table->handle, filter);
+
+	list_for_each_entry_safe(rule, nrule, &ctx->list, list) {
+		chain = chain_cache_find(table, rule->handle.chain.name);
+		if (!chain)
+			chain = chain_binding_lookup(table,
+						     rule->handle.chain.name);
+		if (!chain)
+			return -1;
+
+		list_move_tail(&rule->list, &chain->rules);
+	}
+
+	return ret;
+}
+
 static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 			      const struct nft_cache_filter *filter)
 {
@@ -818,9 +841,7 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 	struct nftnl_chain_list *chain_list = NULL;
 	struct nftnl_set_list *set_list = NULL;
 	struct nftnl_obj_list *obj_list;
-	struct rule *rule, *nrule;
 	struct table *table;
-	struct chain *chain;
 	struct set *set;
 	int ret = 0;
 
@@ -902,19 +923,7 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 		}
 
 		if (flags & NFT_CACHE_RULE_BIT) {
-			ret = rule_cache_init(ctx, &table->handle, filter);
-			list_for_each_entry_safe(rule, nrule, &ctx->list, list) {
-				chain = chain_cache_find(table, rule->handle.chain.name);
-				if (!chain)
-					chain = chain_binding_lookup(table,
-							rule->handle.chain.name);
-				if (!chain) {
-					ret = -1;
-					goto cache_fails;
-				}
-
-				list_move_tail(&rule->list, &chain->rules);
-			}
+			ret = rule_init_cache(ctx, table, filter);
 			if (ret < 0)
 				goto cache_fails;
 		}
-- 
2.30.2

[PATCH nft 4/4] src: 'nft list chain' prints anonymous chains correctly

[Pablo Neira Ayuso]

If the user is requesting a chain listing, e.g. nft list chain x y
and a rule refers to an anonymous chain that cannot be found in the cache,
then fetch such anonymous chain and its ruleset.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1577
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/cache.h                               |  3 ++
 include/netlink.h                             |  1 +
 src/cache.c                                   | 37 +++++++++++++++++++
 src/netlink_delinearize.c                     |  8 ++++
 .../testcases/cache/0010_implicit_chain_0     | 19 ++++++++++
 5 files changed, 68 insertions(+)
 create mode 100755 tests/shell/testcases/cache/0010_implicit_chain_0

diff --git a/include/cache.h b/include/cache.h
index d185f3cfeda0..b6c7d48bfba6 100644
--- a/include/cache.h
+++ b/include/cache.h
@@ -134,4 +134,7 @@ struct nft_cache {
 	uint32_t		flags;
 };
 
+void nft_chain_cache_update(struct netlink_ctx *ctx, struct table *table,
+			    const char *chain);
+
 #endif /* _NFT_CACHE_H_ */
diff --git a/include/netlink.h b/include/netlink.h
index 0e439061e380..e8e0f68ae1a4 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -39,6 +39,7 @@ struct netlink_parse_ctx {
 	struct stmt		*stmt;
 	struct expr		*registers[MAX_REGS + 1];
 	unsigned int		debug_mask;
+	struct netlink_ctx	*nlctx;
 };
 
 struct rule_pp_ctx {
diff --git a/src/cache.c b/src/cache.c
index 14957f2de3a9..630d6ae1307c 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -423,6 +423,21 @@ chain_cache_dump(struct netlink_ctx *ctx,
 	return chain_list;
 }
 
+void nft_chain_cache_update(struct netlink_ctx *ctx, struct table *table,
+			    const char *chain)
+{
+	struct nftnl_chain_list *chain_list;
+
+	chain_list = mnl_nft_chain_dump(ctx, table->handle.family,
+					table->handle.table.name, chain);
+	if (!chain_list)
+		return;
+
+	chain_cache_init(ctx, table, chain_list);
+
+	nftnl_chain_list_free(chain_list);
+}
+
 void chain_cache_add(struct chain *chain, struct table *table)
 {
 	uint32_t hash;
@@ -834,6 +849,22 @@ static int rule_init_cache(struct netlink_ctx *ctx, struct table *table,
 	return ret;
 }
 
+static int implicit_chain_cache(struct netlink_ctx *ctx, struct table *table,
+				const char *chain_name)
+{
+	struct nft_cache_filter filter;
+	struct chain *chain;
+	int ret = 0;
+
+	list_for_each_entry(chain, &table->chain_bindings, cache.list) {
+		filter.list.table = table->handle.table.name;
+		filter.list.chain = chain->handle.chain.name;
+		ret = rule_init_cache(ctx, table, &filter);
+	}
+
+	return ret;
+}
+
 static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 			      const struct nft_cache_filter *filter)
 {
@@ -926,6 +957,12 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags,
 			ret = rule_init_cache(ctx, table, filter);
 			if (ret < 0)
 				goto cache_fails;
+
+			if (filter && filter->list.table && filter->list.chain) {
+				ret = implicit_chain_cache(ctx, table, filter->list.chain);
+				if (ret < 0)
+					goto cache_fails;
+			}
 		}
 	}
 
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 323e9150cdf6..0ea89135cc9a 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -218,6 +218,13 @@ static void netlink_parse_chain_verdict(struct netlink_parse_ctx *ctx,
 
 	expr_chain_export(expr->chain, chain_name);
 	chain = chain_binding_lookup(ctx->table, chain_name);
+
+	/* Special case: 'nft list chain x y' needs to pull in implicit chains */
+	if (!chain && !strncmp(chain_name, "__chain", strlen("__chain"))) {
+		nft_chain_cache_update(ctx->nlctx, ctx->table, chain_name);
+		chain = chain_binding_lookup(ctx->table, chain_name);
+	}
+
 	if (chain) {
 		ctx->stmt = chain_stmt_alloc(loc, chain, verdict);
 		expr_free(expr);
@@ -3141,6 +3148,7 @@ struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
 	memset(&_ctx, 0, sizeof(_ctx));
 	_ctx.msgs = ctx->msgs;
 	_ctx.debug_mask = ctx->nft->debug_mask;
+	_ctx.nlctx = ctx;
 
 	memset(&h, 0, sizeof(h));
 	h.family = nftnl_rule_get_u32(nlr, NFTNL_RULE_FAMILY);
diff --git a/tests/shell/testcases/cache/0010_implicit_chain_0 b/tests/shell/testcases/cache/0010_implicit_chain_0
new file mode 100755
index 000000000000..0ab0db957cf2
--- /dev/null
+++ b/tests/shell/testcases/cache/0010_implicit_chain_0
@@ -0,0 +1,19 @@
+#!/bin/bash
+
+set -e
+
+EXPECTED="table ip f {
+	chain c {
+		jump {
+			accept
+		}
+	}
+}"
+
+$NFT 'table ip f { chain c { jump { accept; }; }; }'
+GET="$($NFT list chain ip f c)"
+
+if [ "$EXPECTED" != "$GET" ] ; then
+	$DIFF -u <(echo "$EXPECTED") <(echo "$GET")
+	exit 1
+fi
-- 
2.30.2