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 <phil@nwl.cc>

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 9a03bbfbb32bbb44a43dfd4bc76453c6e1e57f85..b7f10ab923bc0210c550c7922ee10b72a4c7e690 100644 (file)
--- 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 89dde9ecca779aeba0525776ca5eee208ede4f9f..17e735aa694af7636cfc6206c58c9d307571000b 100644 (file)
--- 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 a7b652ff62a45b7721718ece171f55b70b9b78fe..ef79b018f78360d953daadc2fab6e35cf18153d1 100644 (file)
--- 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 1b85b060c14872e072164fe1103c46dbeb8dab78..cc07e4be31177dd96ac21ce5b5191f0b4c87b92f 100755 (executable)
--- 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 - <<EOF
 table ip filter {
+       chain a {
+               type filter hook input priority filter
+       }
+
         chain INPUT {
                 type filter hook input priority filter
                 counter packets 218 bytes 91375 accept
@@ -24,4 +28,10 @@ table ip filter {
 }
 EOF
 
-$XT_MULTI iptables -L
+EXPECT="# Table \`filter' contains incompatible base-chains, use 'nft' tool to list them.
+-P INPUT ACCEPT
+-P FORWARD ACCEPT
+-P OUTPUT ACCEPT
+-A INPUT -j ACCEPT"
+
+diff -u -Z <(echo -e "$EXPECT") <($XT_MULTI iptables -S)

diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 98cd0ed3f4716d568c17969ba2b4b7b7a5855dd5..f794e3ff1e31854d28466c2f44b1f27708720932 100644 (file)
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -78,6 +78,9 @@ __do_output(struct nft_handle *h, const char *tablename, void *data)
                printf("# Table `%s' is incompatible, use 'nft' tool.\n",
                       tablename);
                return 0;
+       } else if (nft_is_table_tainted(h, tablename)) {
+               printf("# Table `%s' contains incompatible base-chains, use 'nft' tool to list them.\n",
+                      tablename);
        }
 
        now = time(NULL);