All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iptables 1/3] xtables-compat: remove useless functions
@ 2016-08-21 18:10 Pablo M. Bermudo Garay
  2016-08-21 18:10 ` [PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible Pablo M. Bermudo Garay
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Pablo M. Bermudo Garay @ 2016-08-21 18:10 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo, Pablo M. Bermudo Garay

The static function nft_rule_list_get was exposed outside nft.c through
the nft_rule_list_create function, but this was never used out there.

A similar situation occurs with nftnl_rule_list_free and
nft_rule_list_destroy.

This patch removes nft_rule_list_create and nft_rule_list_destroy for
the sake of simplicity.

Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
---
 iptables/nft.c | 38 ++++++++++++++------------------------
 iptables/nft.h |  3 ---
 2 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 05ba57a..247a60a 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1766,16 +1766,6 @@ static int __nft_rule_del(struct nft_handle *h, struct nftnl_rule_list *list,
 	return 1;
 }
 
-struct nftnl_rule_list *nft_rule_list_create(struct nft_handle *h)
-{
-	return nft_rule_list_get(h);
-}
-
-void nft_rule_list_destroy(struct nftnl_rule_list *list)
-{
-	nftnl_rule_list_free(list);
-}
-
 static struct nftnl_rule *
 nft_rule_find(struct nft_handle *h, struct nftnl_rule_list *list,
 	      const char *chain, const char *table, void *data, int rulenum)
@@ -1831,7 +1821,7 @@ int nft_rule_check(struct nft_handle *h, const char *chain,
 
 	nft_fn = nft_rule_check;
 
-	list = nft_rule_list_create(h);
+	list = nft_rule_list_get(h);
 	if (list == NULL)
 		return 0;
 
@@ -1839,7 +1829,7 @@ int nft_rule_check(struct nft_handle *h, const char *chain,
 	if (ret == 0)
 		errno = ENOENT;
 
-	nft_rule_list_destroy(list);
+	nftnl_rule_list_free(list);
 
 	return ret;
 }
@@ -1853,7 +1843,7 @@ int nft_rule_delete(struct nft_handle *h, const char *chain,
 
 	nft_fn = nft_rule_delete;
 
-	list = nft_rule_list_create(h);
+	list = nft_rule_list_get(h);
 	if (list == NULL)
 		return 0;
 
@@ -1865,7 +1855,7 @@ int nft_rule_delete(struct nft_handle *h, const char *chain,
 	} else
 		errno = ENOENT;
 
-	nft_rule_list_destroy(list);
+	nftnl_rule_list_free(list);
 
 	return ret;
 }
@@ -1906,7 +1896,7 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
 	nft_fn = nft_rule_insert;
 
 	if (rulenum > 0) {
-		list = nft_rule_list_create(h);
+		list = nft_rule_list_get(h);
 		if (list == NULL)
 			goto err;
 
@@ -1918,7 +1908,7 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
 			r = nft_rule_find(h, list, chain, table, data,
 					  rulenum - 1);
 			if (r != NULL) {
-				nft_rule_list_destroy(list);
+				nftnl_rule_list_free(list);
 				return nft_rule_append(h, chain, table, data,
 						       0, verbose);
 			}
@@ -1930,12 +1920,12 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
 		handle = nftnl_rule_get_u64(r, NFTNL_RULE_HANDLE);
 		DEBUGP("adding after rule handle %"PRIu64"\n", handle);
 
-		nft_rule_list_destroy(list);
+		nftnl_rule_list_free(list);
 	}
 
 	return nft_rule_add(h, chain, table, data, handle, verbose);
 err:
-	nft_rule_list_destroy(list);
+	nftnl_rule_list_free(list);
 	return 0;
 }
 
@@ -1948,7 +1938,7 @@ int nft_rule_delete_num(struct nft_handle *h, const char *chain,
 
 	nft_fn = nft_rule_delete_num;
 
-	list = nft_rule_list_create(h);
+	list = nft_rule_list_get(h);
 	if (list == NULL)
 		return 0;
 
@@ -1963,7 +1953,7 @@ int nft_rule_delete_num(struct nft_handle *h, const char *chain,
 	} else
 		errno = ENOENT;
 
-	nft_rule_list_destroy(list);
+	nftnl_rule_list_free(list);
 
 	return ret;
 }
@@ -1977,7 +1967,7 @@ int nft_rule_replace(struct nft_handle *h, const char *chain,
 
 	nft_fn = nft_rule_replace;
 
-	list = nft_rule_list_create(h);
+	list = nft_rule_list_get(h);
 	if (list == NULL)
 		return 0;
 
@@ -1993,7 +1983,7 @@ int nft_rule_replace(struct nft_handle *h, const char *chain,
 	} else
 		errno = ENOENT;
 
-	nft_rule_list_destroy(list);
+	nftnl_rule_list_free(list);
 
 	return ret;
 }
@@ -2256,7 +2246,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
 
 	nft_fn = nft_rule_delete;
 
-	list = nft_rule_list_create(h);
+	list = nft_rule_list_get(h);
 	if (list == NULL)
 		return 0;
 
@@ -2276,7 +2266,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
 			       false);
 
 error:
-	nft_rule_list_destroy(list);
+	nftnl_rule_list_free(list);
 
 	return ret;
 }
diff --git a/iptables/nft.h b/iptables/nft.h
index 52f2136..bcabf42 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -92,9 +92,6 @@ int nft_rule_save(struct nft_handle *h, const char *table, bool counters);
 int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table);
 int nft_rule_zero_counters(struct nft_handle *h, const char *chain, const char *table, int rulenum);
 
-struct nftnl_rule_list *nft_rule_list_create(struct nft_handle *h);
-void nft_rule_list_destroy(struct nftnl_rule_list *list);
-
 /*
  * Operations used in userspace tools
  */
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible
  2016-08-21 18:10 [PATCH iptables 1/3] xtables-compat: remove useless functions Pablo M. Bermudo Garay
@ 2016-08-21 18:10 ` Pablo M. Bermudo Garay
  2016-08-21 20:25   ` Arturo Borrero Gonzalez
  2016-08-22  9:56   ` Pablo Neira Ayuso
  2016-08-21 18:10 ` [PATCH iptables 3/3] xtables-compat: add rule cache Pablo M. Bermudo Garay
  2016-08-22  9:51 ` [PATCH iptables 1/3] xtables-compat: remove useless functions Pablo Neira Ayuso
  2 siblings, 2 replies; 6+ messages in thread
From: Pablo M. Bermudo Garay @ 2016-08-21 18:10 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo, Pablo M. Bermudo Garay

This patch adds a verification of the compatibility between the nft
ruleset and iptables. If the nft ruleset is not compatible with
iptables, the execution stops and an error message is displayed to the
user.

This checking is triggered by xtables-compat -L and xtables-compat-save
commands.

Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
---
 iptables/nft.c          | 166 ++++++++++++++++++++++++++++++++++++++++++++++++
 iptables/nft.h          |   2 +
 iptables/xtables-save.c |   5 ++
 iptables/xtables.c      |   5 ++
 4 files changed, 178 insertions(+)

diff --git a/iptables/nft.c b/iptables/nft.c
index 247a60a..7389689 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -2698,3 +2698,169 @@ uint32_t nft_invflags2cmp(uint32_t invflags, uint32_t flag)
 
 	return NFT_CMP_EQ;
 }
+
+static int nft_is_rule_compatible(struct nftnl_rule *rule)
+{
+	struct nftnl_expr_iter *iter;
+	struct nftnl_expr *expr;
+
+	iter = nftnl_expr_iter_create(rule);
+	if (iter == NULL)
+		return -1;
+
+	expr = nftnl_expr_iter_next(iter);
+	while (expr != NULL) {
+		const char *name = nftnl_expr_get_str(expr, NFTNL_EXPR_NAME);
+
+		if (strcmp(name, "counter")   &&
+		    strcmp(name, "match")     &&
+		    strcmp(name, "target")    &&
+		    strcmp(name, "payload")   &&
+		    strcmp(name, "meta")      &&
+		    strcmp(name, "bitwise")   &&
+		    strcmp(name, "cmp")       &&
+		    strcmp(name, "immediate") &&
+		    strcmp(name, "match")     &&
+		    strcmp(name, "target")) {
+			nftnl_expr_iter_destroy(iter);
+			return 1;
+		}
+
+		expr = nftnl_expr_iter_next(iter);
+	}
+
+	nftnl_expr_iter_destroy(iter);
+	return 0;
+}
+
+static int check_builtin_chain(const char *table, const char *chain)
+{
+	const char *cur_table;
+	struct builtin_chain *chains;
+	int i, j;
+
+	for (i = 0; i < TABLES_MAX; i++) {
+		cur_table = xtables_ipv4[i].name;
+		chains = xtables_ipv4[i].chains;
+		if (strcmp(table, cur_table) == 0) {
+			for (j = 0; j < NF_INET_NUMHOOKS && chains[j].name; j++) {
+				if (strcmp(chain, chains[j].name) == 0)
+					return 0;
+			}
+		}
+	}
+
+	return 1;
+}
+
+static int nft_are_chains_compatible(struct nft_handle *h)
+{
+	struct nftnl_chain_list *list;
+	struct nftnl_chain_list_iter *iter;
+	struct nftnl_chain *chain;
+
+	list = nftnl_chain_list_get(h);
+	if (list == NULL)
+		return -1;
+
+	iter = nftnl_chain_list_iter_create(list);
+	if (iter == NULL)
+		return -1;
+
+	chain = nftnl_chain_list_iter_next(iter);
+	while (chain != NULL) {
+		if (nft_chain_builtin(chain)) {
+			const char *table = nftnl_chain_get(chain,
+							    NFTNL_CHAIN_TABLE);
+			const char *name = nftnl_chain_get(chain,
+							   NFTNL_CHAIN_NAME);
+
+			if (check_builtin_chain(table, name) == 1) {
+				nftnl_chain_list_iter_destroy(iter);
+				nftnl_chain_list_free(list);
+				return 1;
+			}
+		}
+
+		chain = nftnl_chain_list_iter_next(iter);
+	}
+
+	nftnl_chain_list_iter_destroy(iter);
+	nftnl_chain_list_free(list);
+	return 0;
+}
+
+static int nft_are_tables_compatible(struct nft_handle *h)
+{
+	struct nftnl_table_list *list;
+	struct nftnl_table_list_iter *iter;
+	struct nftnl_table *table;
+
+	list = nftnl_table_list_get(h);
+	if (list == NULL)
+		return -1;
+
+	iter = nftnl_table_list_iter_create(list);
+	if (iter == NULL)
+		return -1;
+
+	table = nftnl_table_list_iter_next(iter);
+	while (table != NULL) {
+		const char *name = nftnl_table_get(table, NFTNL_TABLE_NAME);
+
+		if (strcmp(name, "filter") &&
+		    strcmp(name, "nat")    &&
+		    strcmp(name, "mangle") &&
+		    strcmp(name, "raw")    &&
+		    strcmp(name, "security")) {
+			nftnl_table_list_iter_destroy(iter);
+			nftnl_table_list_free(list);
+			return 1;
+		}
+
+		table = nftnl_table_list_iter_next(iter);
+	}
+
+	nftnl_table_list_iter_destroy(iter);
+	nftnl_table_list_free(list);
+	return 0;
+}
+
+int nft_is_ruleset_compatible(struct nft_handle *h)
+{
+
+	struct nftnl_rule_list *list;
+	struct nftnl_rule_list_iter *iter;
+	struct nftnl_rule *rule;
+	int ret;
+
+	ret = nft_are_tables_compatible(h);
+	if (ret != 0)
+		return ret;
+
+	ret = nft_are_chains_compatible(h);
+	if (ret != 0)
+		return ret;
+
+	list = nft_rule_list_get(h);
+	if (list == NULL)
+		return -1;
+
+	iter = nftnl_rule_list_iter_create(list);
+	if (iter == NULL)
+		return -1;
+
+	rule = nftnl_rule_list_iter_next(iter);
+	while (rule != NULL) {
+		ret = nft_is_rule_compatible(rule);
+		if (ret != 0) {
+			nftnl_rule_list_iter_destroy(iter);
+			return ret;
+		}
+
+		rule = nftnl_rule_list_iter_next(iter);
+	}
+
+	nftnl_rule_list_iter_destroy(iter);
+	return 0;
+}
diff --git a/iptables/nft.h b/iptables/nft.h
index bcabf42..f5449db 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -181,4 +181,6 @@ int nft_arp_rule_insert(struct nft_handle *h, const char *chain,
 
 void nft_rule_to_arpt_entry(struct nftnl_rule *r, struct arpt_entry *fw);
 
+int nft_is_ruleset_compatible(struct nft_handle *h);
+
 #endif
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 50b5b5a..50ae8a5 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -138,6 +138,11 @@ xtables_save_main(int family, const char *progname, int argc, char *argv[])
 		exit(1);
 	}
 
+	if (nft_is_ruleset_compatible(&h) == 1) {
+		printf("WARNING: You're using features from nft that we cannot map to iptables, please keep using nft.\n\n");
+		exit(EXIT_SUCCESS);
+	}
+
 	if (dump) {
 		do_output(&h, tablename, show_counters);
 		exit(0);
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 48b9c51..ceae441 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -1253,6 +1253,11 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table,
 	case CMD_LIST:
 	case CMD_LIST|CMD_ZERO:
 	case CMD_LIST|CMD_ZERO_NUM:
+		if (nft_is_ruleset_compatible(h) == 1) {
+			printf("WARNING: You're using features from nft that we cannot map to iptables, please keep using nft.\n\n");
+			exit(EXIT_SUCCESS);
+		}
+
 		ret = list_entries(h, p.chain, p.table, p.rulenum,
 				   cs.options & OPT_VERBOSE,
 				   cs.options & OPT_NUMERIC,
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH iptables 3/3] xtables-compat: add rule cache
  2016-08-21 18:10 [PATCH iptables 1/3] xtables-compat: remove useless functions Pablo M. Bermudo Garay
  2016-08-21 18:10 ` [PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible Pablo M. Bermudo Garay
@ 2016-08-21 18:10 ` Pablo M. Bermudo Garay
  2016-08-22  9:51 ` [PATCH iptables 1/3] xtables-compat: remove useless functions Pablo Neira Ayuso
  2 siblings, 0 replies; 6+ messages in thread
From: Pablo M. Bermudo Garay @ 2016-08-21 18:10 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo, Pablo M. Bermudo Garay

This patch adds a cache of rules within the nft handle. This feature is
more useful after the new checks of ruleset compatibility, since the
rule list is loaded twice consecutively.

Now all the operations causing changes in the ruleset must invalidate
the cache, a function called flush_rule_cache has been introduced for
this purpose.

Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
---
 iptables/nft.c | 35 +++++++++++++++++++++++------------
 iptables/nft.h |  1 +
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 7389689..71bf6f7 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -780,8 +780,17 @@ int nft_init(struct nft_handle *h, struct builtin_table *t)
 	return 0;
 }
 
+static void flush_rule_cache(struct nft_handle *h)
+{
+	if (h->rule_cache) {
+		nftnl_rule_list_free(h->rule_cache);
+		h->rule_cache = NULL;
+	}
+}
+
 void nft_fini(struct nft_handle *h)
 {
+	flush_rule_cache(h);
 	mnl_socket_close(h->nl);
 	free(mnl_nlmsg_batch_head(h->batch));
 	mnl_nlmsg_batch_stop(h->batch);
@@ -1121,6 +1130,7 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 	if (batch_rule_add(h, type, r) < 0)
 		nftnl_rule_free(r);
 
+	flush_rule_cache(h);
 	return 1;
 }
 
@@ -1284,6 +1294,9 @@ static struct nftnl_rule_list *nft_rule_list_get(struct nft_handle *h)
 	struct nftnl_rule_list *list;
 	int ret;
 
+	if (h->rule_cache)
+		return h->rule_cache;
+
 	list = nftnl_rule_list_alloc();
 	if (list == NULL)
 		return 0;
@@ -1297,6 +1310,7 @@ static struct nftnl_rule_list *nft_rule_list_get(struct nft_handle *h)
 		return NULL;
 	}
 
+	h->rule_cache = list;
 	return list;
 }
 
@@ -1333,7 +1347,6 @@ next:
 	}
 
 	nftnl_rule_list_iter_destroy(iter);
-	nftnl_rule_list_free(list);
 
 	/* the core expects 1 for success and 0 for error */
 	return 1;
@@ -1396,6 +1409,7 @@ next:
 	}
 
 	nftnl_chain_list_iter_destroy(iter);
+	flush_rule_cache(h);
 err:
 	nftnl_chain_list_free(list);
 
@@ -1829,8 +1843,6 @@ int nft_rule_check(struct nft_handle *h, const char *chain,
 	if (ret == 0)
 		errno = ENOENT;
 
-	nftnl_rule_list_free(list);
-
 	return ret;
 }
 
@@ -1855,7 +1867,7 @@ int nft_rule_delete(struct nft_handle *h, const char *chain,
 	} else
 		errno = ENOENT;
 
-	nftnl_rule_list_free(list);
+	flush_rule_cache(h);
 
 	return ret;
 }
@@ -1879,6 +1891,7 @@ nft_rule_add(struct nft_handle *h, const char *chain,
 		return 0;
 	}
 
+	flush_rule_cache(h);
 	return 1;
 }
 
@@ -1908,7 +1921,7 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
 			r = nft_rule_find(h, list, chain, table, data,
 					  rulenum - 1);
 			if (r != NULL) {
-				nftnl_rule_list_free(list);
+				flush_rule_cache(h);
 				return nft_rule_append(h, chain, table, data,
 						       0, verbose);
 			}
@@ -1920,12 +1933,12 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
 		handle = nftnl_rule_get_u64(r, NFTNL_RULE_HANDLE);
 		DEBUGP("adding after rule handle %"PRIu64"\n", handle);
 
-		nftnl_rule_list_free(list);
+		flush_rule_cache(h);
 	}
 
 	return nft_rule_add(h, chain, table, data, handle, verbose);
 err:
-	nftnl_rule_list_free(list);
+	flush_rule_cache(h);
 	return 0;
 }
 
@@ -1953,7 +1966,7 @@ int nft_rule_delete_num(struct nft_handle *h, const char *chain,
 	} else
 		errno = ENOENT;
 
-	nftnl_rule_list_free(list);
+	flush_rule_cache(h);
 
 	return ret;
 }
@@ -1983,7 +1996,7 @@ int nft_rule_replace(struct nft_handle *h, const char *chain,
 	} else
 		errno = ENOENT;
 
-	nftnl_rule_list_free(list);
+	flush_rule_cache(h);
 
 	return ret;
 }
@@ -2037,8 +2050,6 @@ next:
 
 	nftnl_rule_list_iter_destroy(iter);
 err:
-	nftnl_rule_list_free(list);
-
 	if (ret == 0)
 		errno = ENOENT;
 
@@ -2266,7 +2277,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
 			       false);
 
 error:
-	nftnl_rule_list_free(list);
+	flush_rule_cache(h);
 
 	return ret;
 }
diff --git a/iptables/nft.h b/iptables/nft.h
index f5449db..4126593 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -35,6 +35,7 @@ struct nft_handle {
 	struct mnl_nlmsg_batch	*batch;
 	struct nft_family_ops	*ops;
 	struct builtin_table	*tables;
+	struct nftnl_rule_list	*rule_cache;
 	bool			restore;
 	bool			batch_support;
 };
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible
  2016-08-21 18:10 ` [PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible Pablo M. Bermudo Garay
@ 2016-08-21 20:25   ` Arturo Borrero Gonzalez
  2016-08-22  9:56   ` Pablo Neira Ayuso
  1 sibling, 0 replies; 6+ messages in thread
From: Arturo Borrero Gonzalez @ 2016-08-21 20:25 UTC (permalink / raw)
  To: Pablo M. Bermudo Garay
  Cc: Netfilter Development Mailing list, Pablo Neira Ayuso

On 21 August 2016 at 20:10, Pablo M. Bermudo Garay <pablombg@gmail.com> wrote:
> This patch adds a verification of the compatibility between the nft
> ruleset and iptables. If the nft ruleset is not compatible with
> iptables, the execution stops and an error message is displayed to the
> user.
>
> This checking is triggered by xtables-compat -L and xtables-compat-save
> commands.
>
> Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
> ---
>  iptables/nft.c          | 166 ++++++++++++++++++++++++++++++++++++++++++++++++
>  iptables/nft.h          |   2 +
>  iptables/xtables-save.c |   5 ++
>  iptables/xtables.c      |   5 ++
>  4 files changed, 178 insertions(+)
>
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 247a60a..7389689 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -2698,3 +2698,169 @@ uint32_t nft_invflags2cmp(uint32_t invflags, uint32_t flag)
>
>         return NFT_CMP_EQ;
>  }
> +
> +static int nft_is_rule_compatible(struct nftnl_rule *rule)
> +{
> +       struct nftnl_expr_iter *iter;
> +       struct nftnl_expr *expr;
> +
> +       iter = nftnl_expr_iter_create(rule);
> +       if (iter == NULL)
> +               return -1;
> +
> +       expr = nftnl_expr_iter_next(iter);
> +       while (expr != NULL) {
> +               const char *name = nftnl_expr_get_str(expr, NFTNL_EXPR_NAME);
> +
> +               if (strcmp(name, "counter")   &&
> +                   strcmp(name, "match")     &&
> +                   strcmp(name, "target")    &&
> +                   strcmp(name, "payload")   &&
> +                   strcmp(name, "meta")      &&
> +                   strcmp(name, "bitwise")   &&
> +                   strcmp(name, "cmp")       &&
> +                   strcmp(name, "immediate") &&
> +                   strcmp(name, "match")     &&
> +                   strcmp(name, "target")) {
> +                       nftnl_expr_iter_destroy(iter);
> +                       return 1;
> +               }
> +
> +               expr = nftnl_expr_iter_next(iter);
> +       }
> +
> +       nftnl_expr_iter_destroy(iter);
> +       return 0;
> +}
> +

I don't fully understand this logic. It seems there are expression
names which are repeated. Is that intended?

-- 
Arturo Borrero González

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH iptables 1/3] xtables-compat: remove useless functions
  2016-08-21 18:10 [PATCH iptables 1/3] xtables-compat: remove useless functions Pablo M. Bermudo Garay
  2016-08-21 18:10 ` [PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible Pablo M. Bermudo Garay
  2016-08-21 18:10 ` [PATCH iptables 3/3] xtables-compat: add rule cache Pablo M. Bermudo Garay
@ 2016-08-22  9:51 ` Pablo Neira Ayuso
  2 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-22  9:51 UTC (permalink / raw)
  To: Pablo M. Bermudo Garay; +Cc: netfilter-devel

On Sun, Aug 21, 2016 at 08:10:25PM +0200, Pablo M. Bermudo Garay wrote:
> The static function nft_rule_list_get was exposed outside nft.c through
> the nft_rule_list_create function, but this was never used out there.
> 
> A similar situation occurs with nftnl_rule_list_free and
> nft_rule_list_destroy.
> 
> This patch removes nft_rule_list_create and nft_rule_list_destroy for
> the sake of simplicity.

Applied this patch 1/3. Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible
  2016-08-21 18:10 ` [PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible Pablo M. Bermudo Garay
  2016-08-21 20:25   ` Arturo Borrero Gonzalez
@ 2016-08-22  9:56   ` Pablo Neira Ayuso
  1 sibling, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-22  9:56 UTC (permalink / raw)
  To: Pablo M. Bermudo Garay; +Cc: netfilter-devel

On Sun, Aug 21, 2016 at 08:10:26PM +0200, Pablo M. Bermudo Garay wrote:
> This patch adds a verification of the compatibility between the nft
> ruleset and iptables. If the nft ruleset is not compatible with
> iptables, the execution stops and an error message is displayed to the
> user.

Please, indicate here that this is also checking for built-in tables
and chains.

> This checking is triggered by xtables-compat -L and xtables-compat-save
> commands.
> 
> Signed-off-by: Pablo M. Bermudo Garay <pablombg@gmail.com>
> ---
>  iptables/nft.c          | 166 ++++++++++++++++++++++++++++++++++++++++++++++++
>  iptables/nft.h          |   2 +
>  iptables/xtables-save.c |   5 ++
>  iptables/xtables.c      |   5 ++
>  4 files changed, 178 insertions(+)
> 
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 247a60a..7389689 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -2698,3 +2698,169 @@ uint32_t nft_invflags2cmp(uint32_t invflags, uint32_t flag)
>  
>  	return NFT_CMP_EQ;
>  }
> +
> +static int nft_is_rule_compatible(struct nftnl_rule *rule)
> +{
> +	struct nftnl_expr_iter *iter;
> +	struct nftnl_expr *expr;
> +
> +	iter = nftnl_expr_iter_create(rule);
> +	if (iter == NULL)
> +		return -1;
> +
> +	expr = nftnl_expr_iter_next(iter);
> +	while (expr != NULL) {
> +		const char *name = nftnl_expr_get_str(expr, NFTNL_EXPR_NAME);
> +
> +		if (strcmp(name, "counter")   &&
> +		    strcmp(name, "match")     &&
> +		    strcmp(name, "target")    &&

Arturo is right, this is duplicated.

> +		    strcmp(name, "payload")   &&
> +		    strcmp(name, "meta")      &&
> +		    strcmp(name, "bitwise")   &&
> +		    strcmp(name, "cmp")       &&
> +		    strcmp(name, "immediate") &&
> +		    strcmp(name, "match")     &&
> +		    strcmp(name, "target")) {

I would place this in a array:

#define NFT_COMPAT_EXPR_MAX     8

static const char *supported_exprs[NFT_COMPAT_EXPR] = {
        "match",
        "target"
        "payload",
        "meta"
        "cmp",
        "bitwise",
        "counter",
        "immediate"
};

And make a function for this:

static bool nft_compat_expr(const struct nftnl_expr *expr)
{
        int i;

        for (i = 0; i < NFT_COMPAT_EXPR_MAX; i++) {
                if (!strcmp(supported_exprs[i], name))
                        return true;
        }

        return false;
}

> +			nftnl_expr_iter_destroy(iter);
> +			return 1;
> +		}
> +
> +		expr = nftnl_expr_iter_next(iter);
> +	}
> +
> +	nftnl_expr_iter_destroy(iter);
> +	return 0;
> +}
> +
> +static int check_builtin_chain(const char *table, const char *chain)
> +{
> +	const char *cur_table;
> +	struct builtin_chain *chains;
> +	int i, j;
> +
> +	for (i = 0; i < TABLES_MAX; i++) {
> +		cur_table = xtables_ipv4[i].name;
> +		chains = xtables_ipv4[i].chains;
> +		if (strcmp(table, cur_table) == 0) {
> +			for (j = 0; j < NF_INET_NUMHOOKS && chains[j].name; j++) {
> +				if (strcmp(chain, chains[j].name) == 0)
> +					return 0;
> +			}
> +		}
> +	}
> +
> +	return 1;
> +}
> +
> +static int nft_are_chains_compatible(struct nft_handle *h)
> +{
> +	struct nftnl_chain_list *list;
> +	struct nftnl_chain_list_iter *iter;
> +	struct nftnl_chain *chain;
> +
> +	list = nftnl_chain_list_get(h);
> +	if (list == NULL)
> +		return -1;
> +
> +	iter = nftnl_chain_list_iter_create(list);
> +	if (iter == NULL)
> +		return -1;
> +
> +	chain = nftnl_chain_list_iter_next(iter);
> +	while (chain != NULL) {
> +		if (nft_chain_builtin(chain)) {
> +			const char *table = nftnl_chain_get(chain,
> +							    NFTNL_CHAIN_TABLE);
> +			const char *name = nftnl_chain_get(chain,
> +							   NFTNL_CHAIN_NAME);
> +
> +			if (check_builtin_chain(table, name) == 1) {
> +				nftnl_chain_list_iter_destroy(iter);
> +				nftnl_chain_list_free(list);
> +				return 1;
> +			}
> +		}
> +
> +		chain = nftnl_chain_list_iter_next(iter);
> +	}
> +
> +	nftnl_chain_list_iter_destroy(iter);
> +	nftnl_chain_list_free(list);
> +	return 0;
> +}
> +
> +static int nft_are_tables_compatible(struct nft_handle *h)
> +{
> +	struct nftnl_table_list *list;
> +	struct nftnl_table_list_iter *iter;
> +	struct nftnl_table *table;
> +
> +	list = nftnl_table_list_get(h);
> +	if (list == NULL)
> +		return -1;
> +
> +	iter = nftnl_table_list_iter_create(list);
> +	if (iter == NULL)
> +		return -1;
> +
> +	table = nftnl_table_list_iter_next(iter);
> +	while (table != NULL) {
> +		const char *name = nftnl_table_get(table, NFTNL_TABLE_NAME);
> +
> +		if (strcmp(name, "filter") &&
> +		    strcmp(name, "nat")    &&
> +		    strcmp(name, "mangle") &&
> +		    strcmp(name, "raw")    &&
> +		    strcmp(name, "security")) {

Why not use the builtin structure definition for this?

> +			nftnl_table_list_iter_destroy(iter);
> +			nftnl_table_list_free(list);
> +			return 1;
> +		}
> +
> +		table = nftnl_table_list_iter_next(iter);
> +	}
> +
> +	nftnl_table_list_iter_destroy(iter);
> +	nftnl_table_list_free(list);
> +	return 0;
> +}
> +
> +int nft_is_ruleset_compatible(struct nft_handle *h)
> +{
> +
> +	struct nftnl_rule_list *list;
> +	struct nftnl_rule_list_iter *iter;
> +	struct nftnl_rule *rule;
> +	int ret;
> +
> +	ret = nft_are_tables_compatible(h);
> +	if (ret != 0)
> +		return ret;
> +
> +	ret = nft_are_chains_compatible(h);
> +	if (ret != 0)
> +		return ret;
> +
> +	list = nft_rule_list_get(h);
> +	if (list == NULL)
> +		return -1;
> +
> +	iter = nftnl_rule_list_iter_create(list);
> +	if (iter == NULL)
> +		return -1;
> +
> +	rule = nftnl_rule_list_iter_next(iter);
> +	while (rule != NULL) {
> +		ret = nft_is_rule_compatible(rule);
> +		if (ret != 0) {
> +			nftnl_rule_list_iter_destroy(iter);
> +			return ret;
> +		}
> +
> +		rule = nftnl_rule_list_iter_next(iter);
> +	}
> +
> +	nftnl_rule_list_iter_destroy(iter);
> +	return 0;
> +}
> diff --git a/iptables/nft.h b/iptables/nft.h
> index bcabf42..f5449db 100644
> --- a/iptables/nft.h
> +++ b/iptables/nft.h
> @@ -181,4 +181,6 @@ int nft_arp_rule_insert(struct nft_handle *h, const char *chain,
>  
>  void nft_rule_to_arpt_entry(struct nftnl_rule *r, struct arpt_entry *fw);
>  
> +int nft_is_ruleset_compatible(struct nft_handle *h);
> +
>  #endif
> diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
> index 50b5b5a..50ae8a5 100644
> --- a/iptables/xtables-save.c
> +++ b/iptables/xtables-save.c
> @@ -138,6 +138,11 @@ xtables_save_main(int family, const char *progname, int argc, char *argv[])
>  		exit(1);
>  	}
>  
> +	if (nft_is_ruleset_compatible(&h) == 1) {
> +		printf("WARNING: You're using features from nft that we cannot map to iptables, please keep using nft.\n\n");
> +		exit(EXIT_SUCCESS);

Success? :) I would say ERROR here and get back to the shell with
EXIT_FAILURE. Same thing in other spots.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-08-22  9:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-21 18:10 [PATCH iptables 1/3] xtables-compat: remove useless functions Pablo M. Bermudo Garay
2016-08-21 18:10 ` [PATCH iptables 2/3] xtables-compat: check if nft ruleset is compatible Pablo M. Bermudo Garay
2016-08-21 20:25   ` Arturo Borrero Gonzalez
2016-08-22  9:56   ` Pablo Neira Ayuso
2016-08-21 18:10 ` [PATCH iptables 3/3] xtables-compat: add rule cache Pablo M. Bermudo Garay
2016-08-22  9:51 ` [PATCH iptables 1/3] xtables-compat: remove useless functions Pablo Neira Ayuso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.