From f9b33967f2b4b58160c0a970da77d5e44406803a Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Tue, 21 Sep 2021 16:42:36 +0200 Subject: nft: Check base-chain compatibility when adding to cache With introduction of dedicated base-chain slots, a selection process was established as no longer all base-chains ended in the same chain list for later searching/checking but only the first one found for each hook matching criteria is kept and the rest discarded. A side-effect of the above is that table compatibility checking started to omit consecutive base-chains, making iptables-nft less restrictive as long as the expected base-chains were returned first from kernel when populating the cache. Make behaviour consistent and warn users about the possibly disturbing chains found by: * Run all base-chain checks from nft_is_chain_compatible() before allowing a base-chain to occupy its slot. * If an unfit base-chain was found (and discarded), flag the table's cache as tainted and warn about it if the remaining ruleset is otherwise compatible. Since base-chains that remain in cache would pass nft_is_chain_compatible() checking, remove that and reduce it to rule inspection. Signed-off-by: Phil Sutter --- iptables/nft-cache.c | 47 +++++++++++++++++----- iptables/nft.c | 45 ++++++--------------- iptables/nft.h | 2 + .../tests/shell/testcases/chain/0004extra-base_0 | 12 +++++- iptables/xtables-save.c | 3 ++ 5 files changed, 65 insertions(+), 44 deletions(-) (limited to 'iptables') diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c index 9a03bbfb..b7f10ab9 100644 --- a/iptables/nft-cache.c +++ b/iptables/nft-cache.c @@ -202,26 +202,51 @@ nft_chain_find(struct nft_handle *h, const char *table, const char *chain) return NULL; } +static int +nft_cache_add_base_chain(struct nft_handle *h, const struct builtin_table *t, + struct nft_chain *nc) +{ + uint32_t hooknum = nftnl_chain_get_u32(nc->nftnl, NFTNL_CHAIN_HOOKNUM); + const char *name = nftnl_chain_get_str(nc->nftnl, NFTNL_CHAIN_NAME); + const char *type = nftnl_chain_get_str(nc->nftnl, NFTNL_CHAIN_TYPE); + uint32_t prio = nftnl_chain_get_u32(nc->nftnl, NFTNL_CHAIN_PRIO); + const struct builtin_chain *bc = NULL; + int i; + + for (i = 0; i < NF_IP_NUMHOOKS && t->chains[i].name != NULL; i++) { + if (hooknum == t->chains[i].hook) { + bc = &t->chains[i]; + break; + } + } + + if (!bc || + prio != bc->prio || + strcmp(name, bc->name) || + strcmp(type, bc->type)) + return -EINVAL; + + if (h->cache->table[t->type].base_chains[hooknum]) + return -EEXIST; + + h->cache->table[t->type].base_chains[hooknum] = nc; + return 0; +} + int nft_cache_add_chain(struct nft_handle *h, const struct builtin_table *t, struct nftnl_chain *c) { const char *cname = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME); struct nft_chain *nc = nft_chain_alloc(c); + int ret; if (nftnl_chain_is_set(c, NFTNL_CHAIN_HOOKNUM)) { - uint32_t hooknum = nftnl_chain_get_u32(c, NFTNL_CHAIN_HOOKNUM); - - if (hooknum >= NF_INET_NUMHOOKS) { + ret = nft_cache_add_base_chain(h, t, nc); + if (ret) { + h->cache->table[t->type].tainted = true; nft_chain_free(nc); - return -EINVAL; + return ret; } - - if (h->cache->table[t->type].base_chains[hooknum]) { - nft_chain_free(nc); - return -EEXIST; - } - - h->cache->table[t->type].base_chains[hooknum] = nc; } else { list_add_tail(&nc->head, &h->cache->table[t->type].chains->list); diff --git a/iptables/nft.c b/iptables/nft.c index 89dde9ec..17e735aa 100644 --- a/iptables/nft.c +++ b/iptables/nft.c @@ -3513,38 +3513,8 @@ static int nft_is_rule_compatible(struct nftnl_rule *rule, void *data) static int nft_is_chain_compatible(struct nft_chain *nc, void *data) { struct nftnl_chain *c = nc->nftnl; - const struct builtin_table *table; - const struct builtin_chain *chain; - const char *tname, *cname, *type; - struct nft_handle *h = data; - enum nf_inet_hooks hook; - int prio; - - if (nftnl_rule_foreach(c, nft_is_rule_compatible, NULL)) - return -1; - - if (!nft_chain_builtin(c)) - return 0; - - tname = nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE); - table = nft_table_builtin_find(h, tname); - if (!table) - return -1; - - cname = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME); - chain = nft_chain_builtin_find(table, cname); - if (!chain) - return -1; - type = nftnl_chain_get_str(c, NFTNL_CHAIN_TYPE); - prio = nftnl_chain_get_u32(c, NFTNL_CHAIN_PRIO); - hook = nftnl_chain_get_u32(c, NFTNL_CHAIN_HOOKNUM); - if (strcmp(type, chain->type) || - prio != chain->prio || - hook != chain->hook) - return -1; - - return 0; + return nftnl_rule_foreach(c, nft_is_rule_compatible, NULL); } bool nft_is_table_compatible(struct nft_handle *h, @@ -3559,13 +3529,24 @@ bool nft_is_table_compatible(struct nft_handle *h, return !nft_chain_foreach(h, table, nft_is_chain_compatible, h); } +bool nft_is_table_tainted(struct nft_handle *h, const char *table) +{ + const struct builtin_table *t = nft_table_builtin_find(h, table); + + return t ? h->cache->table[t->type].tainted : false; +} + void nft_assert_table_compatible(struct nft_handle *h, const char *table, const char *chain) { const char *pfx = "", *sfx = ""; - if (nft_is_table_compatible(h, table, chain)) + if (nft_is_table_compatible(h, table, chain)) { + if (nft_is_table_tainted(h, table)) + printf("# Table `%s' contains incompatible base-chains, use 'nft' tool to list them.\n", + table); return; + } if (chain) { pfx = "chain `"; diff --git a/iptables/nft.h b/iptables/nft.h index a7b652ff..ef79b018 100644 --- a/iptables/nft.h +++ b/iptables/nft.h @@ -45,6 +45,7 @@ struct nft_cache { struct nftnl_set_list *sets; bool exists; bool sorted; + bool tainted; } table[NFT_TABLE_MAX]; }; @@ -262,6 +263,7 @@ void nft_rule_to_arpt_entry(struct nftnl_rule *r, struct arpt_entry *fw); bool nft_is_table_compatible(struct nft_handle *h, const char *table, const char *chain); +bool nft_is_table_tainted(struct nft_handle *h, const char *table); void nft_assert_table_compatible(struct nft_handle *h, const char *table, const char *chain); diff --git a/iptables/tests/shell/testcases/chain/0004extra-base_0 b/iptables/tests/shell/testcases/chain/0004extra-base_0 index 1b85b060..cc07e4be 100755 --- a/iptables/tests/shell/testcases/chain/0004extra-base_0 +++ b/iptables/tests/shell/testcases/chain/0004extra-base_0 @@ -13,6 +13,10 @@ set -e nft -f - <