All of lore.kernel.org
 help / color / mirror / Atom feed
* [nft PATCH v2 0/4] xt: Implement dump and restore support
@ 2022-11-17 17:45 Phil Sutter
  2022-11-17 17:45 ` [nft PATCH v2 1/4] xt: Delay libxtables access until translation Phil Sutter
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Phil Sutter @ 2022-11-17 17:45 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

If nft can't translate a compat expression, dump it in a format that can
be restored later without losing data, thereby keeping the ruleset
intact.

Patch 1 is preparation (more or less), patch 2 has the gory details,
patch 3 is a minor code refactoring that's almost unrelated and patch 4
further sanitizes behaviour now that there's a reliable fallback in
place.

Changes since v1:
- Use patch 3 to also improve the error printing if extension lookup
  fails.
- New patch 4.

Phil Sutter (4):
  xt: Delay libxtables access until translation
  xt: Implement dump and restore support
  xt: Put match/target translation into own functions
  xt: Detect xlate callback failure

 configure.ac              |  12 +-
 doc/libnftables-json.adoc |  15 +-
 doc/statements.txt        |  17 ++
 include/base64.h          |  17 ++
 include/json.h            |   2 +
 include/parser.h          |   1 +
 include/statement.h       |   9 +-
 include/xt.h              |   4 +
 src/Makefile.am           |   3 +-
 src/base64.c              | 170 ++++++++++++++++++++
 src/evaluate.c            |   1 +
 src/json.c                |  25 ++-
 src/netlink_linearize.c   |  32 ++++
 src/parser_bison.y        |  28 ++++
 src/parser_json.c         |  36 +++++
 src/scanner.l             |  14 ++
 src/statement.c           |   1 +
 src/xt.c                  | 317 ++++++++++++++++++++++----------------
 18 files changed, 558 insertions(+), 146 deletions(-)
 create mode 100644 include/base64.h
 create mode 100644 src/base64.c

-- 
2.38.0


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

* [nft PATCH v2 1/4] xt: Delay libxtables access until translation
  2022-11-17 17:45 [nft PATCH v2 0/4] xt: Implement dump and restore support Phil Sutter
@ 2022-11-17 17:45 ` Phil Sutter
  2022-11-17 17:45 ` [nft PATCH v2 2/4] xt: Implement dump and restore support Phil Sutter
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Phil Sutter @ 2022-11-17 17:45 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

There is no point in spending efforts setting up the xt match/target
when it is not printed afterwards. So just store the statement data from
libnftnl in struct xt_stmt and perform the extension lookup from
xt_stmt_xlate() instead.

This means some data structures are only temporarily allocated for the
sake of passing to libxtables callbacks, no need to drag them around.
Also no need to clone the looked up extension, it is needed only to call
the functions it provides.

While being at it, select numeric output in xt_xlate_*_params -
otherwise there will be reverse DNS lookups which should not happen by
default.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/statement.h |   9 +--
 src/xt.c            | 192 ++++++++++++++++++--------------------------
 2 files changed, 80 insertions(+), 121 deletions(-)

diff --git a/include/statement.h b/include/statement.h
index 2a2d300106181..8651fc78892c9 100644
--- a/include/statement.h
+++ b/include/statement.h
@@ -264,12 +264,11 @@ struct xtables_target;
 struct xt_stmt {
 	const char			*name;
 	enum nft_xt_type		type;
+	uint32_t			rev;
+	uint32_t			family;
+	size_t				infolen;
+	void				*info;
 	uint32_t			proto;
-	union {
-		struct xtables_match	*match;
-		struct xtables_target	*target;
-	};
-	void				*entry;
 };
 
 extern struct stmt *xt_stmt_alloc(const struct location *loc);
diff --git a/src/xt.c b/src/xt.c
index 789de9926261b..f14f40157ba10 100644
--- a/src/xt.c
+++ b/src/xt.c
@@ -28,51 +28,94 @@
 
 #ifdef HAVE_LIBXTABLES
 #include <xtables.h>
+
+static void *xt_entry_alloc(const struct xt_stmt *xt, uint32_t af);
 #endif
 
 void xt_stmt_xlate(const struct stmt *stmt, struct output_ctx *octx)
 {
 #ifdef HAVE_LIBXTABLES
 	struct xt_xlate *xl = xt_xlate_alloc(10240);
+	struct xtables_target *tg;
+	struct xt_entry_target *t;
+	struct xtables_match *mt;
+	struct xt_entry_match *m;
+	size_t size;
+	void *entry;
+
+	xtables_set_nfproto(stmt->xt.family);
+	entry = xt_entry_alloc(&stmt->xt, stmt->xt.family);
 
 	switch (stmt->xt.type) {
 	case NFT_XT_MATCH:
-		if (stmt->xt.match->xlate) {
+		mt = xtables_find_match(stmt->xt.name, XTF_TRY_LOAD, NULL);
+		if (!mt) {
+			fprintf(stderr, "XT match %s not found\n",
+				stmt->xt.name);
+			return;
+		}
+		size = XT_ALIGN(sizeof(*m)) + stmt->xt.infolen;
+
+		m = xzalloc(size);
+		memcpy(&m->data, stmt->xt.info, stmt->xt.infolen);
+
+		m->u.match_size = size;
+		m->u.user.revision = stmt->xt.rev;
+
+		if (mt->xlate) {
 			struct xt_xlate_mt_params params = {
-				.ip		= stmt->xt.entry,
-				.match		= stmt->xt.match->m,
-				.numeric        = 0,
+				.ip		= entry,
+				.match		= m,
+				.numeric        = 1,
 			};
 
-			stmt->xt.match->xlate(xl, &params);
+			mt->xlate(xl, &params);
 			nft_print(octx, "%s", xt_xlate_get(xl));
-		} else if (stmt->xt.match->print) {
+		} else if (mt->print) {
 			printf("#");
-			stmt->xt.match->print(&stmt->xt.entry,
-					      stmt->xt.match->m, 0);
+			mt->print(&entry, m, 0);
 		}
+		xfree(m);
 		break;
 	case NFT_XT_WATCHER:
 	case NFT_XT_TARGET:
-		if (stmt->xt.target->xlate) {
+		tg = xtables_find_target(stmt->xt.name, XTF_TRY_LOAD);
+		if (!tg) {
+			fprintf(stderr, "XT target %s not found\n",
+				stmt->xt.name);
+			return;
+		}
+		size = XT_ALIGN(sizeof(*t)) + stmt->xt.infolen;
+
+		t = xzalloc(size);
+		memcpy(&t->data, stmt->xt.info, stmt->xt.infolen);
+
+		t->u.target_size = size;
+		t->u.user.revision = stmt->xt.rev;
+
+		strcpy(t->u.user.name, tg->name);
+
+		if (tg->xlate) {
 			struct xt_xlate_tg_params params = {
-				.ip		= stmt->xt.entry,
-				.target		= stmt->xt.target->t,
-				.numeric        = 0,
+				.ip		= entry,
+				.target		= t,
+				.numeric        = 1,
 			};
 
-			stmt->xt.target->xlate(xl, &params);
+			tg->xlate(xl, &params);
 			nft_print(octx, "%s", xt_xlate_get(xl));
-		} else if (stmt->xt.target->print) {
+		} else if (tg->print) {
 			printf("#");
-			stmt->xt.target->print(NULL, stmt->xt.target->t, 0);
+			tg->print(NULL, t, 0);
 		}
+		xfree(t);
 		break;
 	default:
 		break;
 	}
 
 	xt_xlate_free(xl);
+	xfree(entry);
 #else
 	nft_print(octx, "# xt_%s", stmt->xt.name);
 #endif
@@ -80,33 +123,12 @@ void xt_stmt_xlate(const struct stmt *stmt, struct output_ctx *octx)
 
 void xt_stmt_destroy(struct stmt *stmt)
 {
-#ifdef HAVE_LIBXTABLES
-	switch (stmt->xt.type) {
-	case NFT_XT_MATCH:
-		if (!stmt->xt.match)
-			break;
-		if (stmt->xt.match->m)
-			xfree(stmt->xt.match->m);
-		xfree(stmt->xt.match);
-		break;
-	case NFT_XT_WATCHER:
-	case NFT_XT_TARGET:
-		if (!stmt->xt.target)
-			break;
-		if (stmt->xt.target->t)
-			xfree(stmt->xt.target->t);
-		xfree(stmt->xt.target);
-		break;
-	default:
-		break;
-	}
-#endif
-	xfree(stmt->xt.entry);
 	xfree(stmt->xt.name);
+	xfree(stmt->xt.info);
 }
 
 #ifdef HAVE_LIBXTABLES
-static void *xt_entry_alloc(struct xt_stmt *xt, uint32_t af)
+static void *xt_entry_alloc(const struct xt_stmt *xt, uint32_t af)
 {
 	union nft_entry {
 		struct ipt_entry ipt;
@@ -173,24 +195,6 @@ static uint32_t xt_proto(const struct proto_ctx *pctx)
 
 	return 0;
 }
-
-static struct xtables_target *xt_target_clone(struct xtables_target *t)
-{
-	struct xtables_target *clone;
-
-	clone = xzalloc(sizeof(struct xtables_target));
-	memcpy(clone, t, sizeof(struct xtables_target));
-	return clone;
-}
-
-static struct xtables_match *xt_match_clone(struct xtables_match *m)
-{
-	struct xtables_match *clone;
-
-	clone = xzalloc(sizeof(struct xtables_match));
-	memcpy(clone, m, sizeof(struct xtables_match));
-	return clone;
-}
 #endif
 
 /*
@@ -201,43 +205,22 @@ void netlink_parse_match(struct netlink_parse_ctx *ctx,
 			 const struct location *loc,
 			 const struct nftnl_expr *nle)
 {
-	struct stmt *stmt;
-	const char *name;
-#ifdef HAVE_LIBXTABLES
-	struct xtables_match *mt;
 	const char *mtinfo;
-	struct xt_entry_match *m;
+	struct stmt *stmt;
 	uint32_t mt_len;
 
-	xtables_set_nfproto(ctx->table->handle.family);
-
-	name = nftnl_expr_get_str(nle, NFTNL_EXPR_MT_NAME);
-
-	mt = xtables_find_match(name, XTF_TRY_LOAD, NULL);
-	if (!mt) {
-		fprintf(stderr, "XT match %s not found\n", name);
-		return;
-	}
 	mtinfo = nftnl_expr_get(nle, NFTNL_EXPR_MT_INFO, &mt_len);
 
-	m = xzalloc(sizeof(struct xt_entry_match) + mt_len);
-	memcpy(&m->data, mtinfo, mt_len);
-
-	m->u.match_size = mt_len + XT_ALIGN(sizeof(struct xt_entry_match));
-	m->u.user.revision = nftnl_expr_get_u32(nle, NFTNL_EXPR_MT_REV);
-
 	stmt = xt_stmt_alloc(loc);
-	stmt->xt.name = strdup(name);
+	stmt->xt.name = strdup(nftnl_expr_get_str(nle, NFTNL_EXPR_MT_NAME));
 	stmt->xt.type = NFT_XT_MATCH;
-	stmt->xt.match = xt_match_clone(mt);
-	stmt->xt.match->m = m;
-#else
-	name = nftnl_expr_get_str(nle, NFTNL_EXPR_MT_NAME);
+	stmt->xt.rev = nftnl_expr_get_u32(nle, NFTNL_EXPR_MT_REV);
+	stmt->xt.family = ctx->table->handle.family;
+
+	stmt->xt.infolen = mt_len;
+	stmt->xt.info = xmalloc(mt_len);
+	memcpy(stmt->xt.info, mtinfo, mt_len);
 
-	stmt = xt_stmt_alloc(loc);
-	stmt->xt.name = strdup(name);
-	stmt->xt.type = NFT_XT_MATCH;
-#endif
 	rule_stmt_append(ctx->rule, stmt);
 }
 
@@ -245,44 +228,22 @@ void netlink_parse_target(struct netlink_parse_ctx *ctx,
 			  const struct location *loc,
 			  const struct nftnl_expr *nle)
 {
-	struct stmt *stmt;
-	const char *name;
-#ifdef HAVE_LIBXTABLES
-	struct xtables_target *tg;
 	const void *tginfo;
-	struct xt_entry_target *t;
-	size_t size;
+	struct stmt *stmt;
 	uint32_t tg_len;
 
-	xtables_set_nfproto(ctx->table->handle.family);
-
-	name = nftnl_expr_get_str(nle, NFTNL_EXPR_TG_NAME);
-	tg = xtables_find_target(name, XTF_TRY_LOAD);
-	if (!tg) {
-		fprintf(stderr, "XT target %s not found\n", name);
-		return;
-	}
 	tginfo = nftnl_expr_get(nle, NFTNL_EXPR_TG_INFO, &tg_len);
 
-	size = XT_ALIGN(sizeof(struct xt_entry_target)) + tg_len;
-	t = xzalloc(size);
-	memcpy(&t->data, tginfo, tg_len);
-	t->u.target_size = size;
-	t->u.user.revision = nftnl_expr_get_u32(nle, NFTNL_EXPR_TG_REV);
-	strcpy(t->u.user.name, tg->name);
-
 	stmt = xt_stmt_alloc(loc);
-	stmt->xt.name = strdup(name);
+	stmt->xt.name = strdup(nftnl_expr_get_str(nle, NFTNL_EXPR_TG_NAME));
 	stmt->xt.type = NFT_XT_TARGET;
-	stmt->xt.target = xt_target_clone(tg);
-	stmt->xt.target->t = t;
-#else
-	name = nftnl_expr_get_str(nle, NFTNL_EXPR_TG_NAME);
+	stmt->xt.rev = nftnl_expr_get_u32(nle, NFTNL_EXPR_TG_REV);
+	stmt->xt.family = ctx->table->handle.family;
+
+	stmt->xt.infolen = tg_len;
+	stmt->xt.info = xmalloc(tg_len);
+	memcpy(stmt->xt.info, tginfo, tg_len);
 
-	stmt = xt_stmt_alloc(loc);
-	stmt->xt.name = strdup(name);
-	stmt->xt.type = NFT_XT_TARGET;
-#endif
 	rule_stmt_append(ctx->rule, stmt);
 }
 
@@ -309,7 +270,6 @@ void stmt_xt_postprocess(struct rule_pp_ctx *rctx, struct stmt *stmt,
 		stmt->xt.type = NFT_XT_WATCHER;
 
 	stmt->xt.proto = xt_proto(&rctx->pctx);
-	stmt->xt.entry = xt_entry_alloc(&stmt->xt, rctx->pctx.family);
 }
 
 static int nft_xt_compatible_revision(const char *name, uint8_t rev, int opt)
-- 
2.38.0


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

* [nft PATCH v2 2/4] xt: Implement dump and restore support
  2022-11-17 17:45 [nft PATCH v2 0/4] xt: Implement dump and restore support Phil Sutter
  2022-11-17 17:45 ` [nft PATCH v2 1/4] xt: Delay libxtables access until translation Phil Sutter
@ 2022-11-17 17:45 ` Phil Sutter
  2022-11-17 17:45 ` [nft PATCH v2 3/4] xt: Put match/target translation into own functions Phil Sutter
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Phil Sutter @ 2022-11-17 17:45 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

When listing a rule with compat expressions, fall back to a textual
representation which does not require knowledge of the underlying data
structures (i.e. libxtables support), preserves information and gives
some clue to users what it is.

The statement is printed with its type and name in clear text, the
remaining private data and revision encoded as base64 string. To prevent
listings from becoming a total mess, the data is zipped before encoding
(if zlib is available).

Base64 en-/decoding implementation copied from FreeBSD and slightly
adjusted to fit our needs.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 configure.ac              |  12 ++-
 doc/libnftables-json.adoc |  15 +++-
 doc/statements.txt        |  17 ++++
 include/base64.h          |  17 ++++
 include/json.h            |   2 +
 include/parser.h          |   1 +
 include/xt.h              |   4 +
 src/Makefile.am           |   3 +-
 src/base64.c              | 170 ++++++++++++++++++++++++++++++++++++++
 src/evaluate.c            |   1 +
 src/json.c                |  25 ++++--
 src/netlink_linearize.c   |  32 +++++++
 src/parser_bison.y        |  28 +++++++
 src/parser_json.c         |  36 ++++++++
 src/scanner.l             |  14 ++++
 src/statement.c           |   1 +
 src/xt.c                  |  99 ++++++++++++++++++++--
 17 files changed, 458 insertions(+), 19 deletions(-)
 create mode 100644 include/base64.h
 create mode 100644 src/base64.c

diff --git a/configure.ac b/configure.ac
index eb1882dd828e8..268255cfeb39e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -98,6 +98,15 @@ AC_DEFINE([HAVE_LIBXTABLES], [1], [0])
 ])
 AM_CONDITIONAL([BUILD_XTABLES], [test "x$with_xtables" = xyes])
 
+AC_ARG_WITH([zlib], [AS_HELP_STRING([--without-zlib],
+            [Disable payload compression of xt compat expressions when listing])],
+	    [], [with_zlib=yes])
+AS_IF([test "x$with_zlib" != xno], [
+AC_CHECK_LIB([z], [compress], ,
+	     AC_MSG_ERROR([No suitable version of zlib found]))
+AC_DEFINE([HAVE_ZLIB], [1], [Define if you have zlib])
+])
+
 AC_ARG_WITH([json], [AS_HELP_STRING([--with-json],
             [Enable JSON output support])],
 	    [], [with_json=no])
@@ -156,7 +165,8 @@ echo "
   use mini-gmp:			${with_mini_gmp}
   enable man page:              ${enable_man_doc}
   libxtables support:		${with_xtables}
-  json output support:          ${with_json}"
+  json output support:          ${with_json}
+  compress xt compat payloads:  ${with_zlib}"
 
 AS_IF([test "$enable_python" != "no"], [
 	echo "  enable Python:		yes (with $PYTHON_BIN)"
diff --git a/doc/libnftables-json.adoc b/doc/libnftables-json.adoc
index bb59945fc510d..6d75a9b6ed5d3 100644
--- a/doc/libnftables-json.adoc
+++ b/doc/libnftables-json.adoc
@@ -1059,10 +1059,19 @@ Assign connection tracking expectation.
 
 === XT
 [verse]
-*{ "xt": null }*
+____
+*{ "xt": [* 'TYPE' *,* 'STRING' *,* 'STRING' *] }*
+
+'TYPE' := *match* | *target* | *watcher*
+____
+
+This represents an xt statement from xtables compat interface. It merely exists
+to support saving and restoring a ruleset containing these statements without
+losing data.
 
-This represents an xt statement from xtables compat interface. Sadly, at this
-point, it is not possible to provide any further information about its content.
+The textual representation was chosen to give users a rough idea of what it is
+by explicitly stating the extension's 'TYPE' and name in the first two array
+fields. The third one embeds the extension's data in compressed base64 format.
 
 == EXPRESSIONS
 Expressions are the building blocks of (most) statements. In their most basic
diff --git a/doc/statements.txt b/doc/statements.txt
index 8076c21cded41..19ea23d1129f3 100644
--- a/doc/statements.txt
+++ b/doc/statements.txt
@@ -783,3 +783,20 @@ ____
 # jump to different chains depending on layer 4 protocol type:
 nft add rule ip filter input ip protocol vmap { tcp : jump tcp-chain, udp : jump udp-chain , icmp : jump icmp-chain }
 ------------------------
+
+XT STATEMENT
+~~~~~~~~~~~~
+This represents an xt statement from xtables compat interface. It merely exists
+to support saving and restoring a ruleset containing these statements without
+losing data.
+
+[verse]
+____
+*xt* 'TYPE' 'NAME' 'BASE64'
+
+'TYPE' := *match* | *target* | *watcher*
+____
+
+The textual representation was chosen to give users a rough idea of what it is
+by explicitly stating the extension's 'TYPE' and 'NAME' followed by the
+extension's data in compressed 'BASE64' format.
diff --git a/include/base64.h b/include/base64.h
new file mode 100644
index 0000000000000..aa21fd0fc1b76
--- /dev/null
+++ b/include/base64.h
@@ -0,0 +1,17 @@
+/*
+ * Base64 encoding/decoding (RFC1341)
+ * Copyright (c) 2005, Jouni Malinen <j@w1.fi>
+ *
+ * This software may be distributed under the terms of the BSD license.
+ * See README for more details.
+ */
+
+#ifndef BASE64_H
+#define BASE64_H
+
+unsigned char * base64_encode(const unsigned char *src, size_t len,
+			      size_t *out_len);
+unsigned char * base64_decode(const unsigned char *src, size_t len,
+			      size_t *out_len);
+
+#endif /* BASE64_H */
diff --git a/include/json.h b/include/json.h
index b0d78eb84987e..f691678d4d726 100644
--- a/include/json.h
+++ b/include/json.h
@@ -92,6 +92,7 @@ json_t *connlimit_stmt_json(const struct stmt *stmt, struct output_ctx *octx);
 json_t *tproxy_stmt_json(const struct stmt *stmt, struct output_ctx *octx);
 json_t *synproxy_stmt_json(const struct stmt *stmt, struct output_ctx *octx);
 json_t *optstrip_stmt_json(const struct stmt *stmt, struct output_ctx *octx);
+json_t *xt_stmt_json(const struct stmt *stmt, struct output_ctx *octx);
 
 int do_command_list_json(struct netlink_ctx *ctx, struct cmd *cmd);
 
@@ -194,6 +195,7 @@ STMT_PRINT_STUB(connlimit)
 STMT_PRINT_STUB(tproxy)
 STMT_PRINT_STUB(synproxy)
 STMT_PRINT_STUB(optstrip)
+STMT_PRINT_STUB(xt)
 
 #undef STMT_PRINT_STUB
 #undef EXPR_PRINT_STUB
diff --git a/include/parser.h b/include/parser.h
index f55da0fd47bf2..ec5b406ef89c5 100644
--- a/include/parser.h
+++ b/include/parser.h
@@ -83,6 +83,7 @@ enum startcond_type {
 	PARSER_SC_STMT_REJECT,
 	PARSER_SC_STMT_SYNPROXY,
 	PARSER_SC_STMT_TPROXY,
+	PARSER_SC_STMT_XT,
 
 	__SC_MAX
 };
diff --git a/include/xt.h b/include/xt.h
index 9fc515084d597..d365c58479e94 100644
--- a/include/xt.h
+++ b/include/xt.h
@@ -26,4 +26,8 @@ static inline void stmt_xt_postprocess(struct rule_pp_ctx *rctx,
 
 #endif
 
+unsigned char *xt_stmt_blob_encode(const struct stmt *stmt);
+int xt_stmt_blob_decode(struct stmt *stmt, const char *b64_string,
+			const struct location *b64_loc, struct list_head *msgs);
+
 #endif /* _NFT_XT_H_ */
diff --git a/src/Makefile.am b/src/Makefile.am
index 264d981e20c7f..cea5b8df20a51 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -36,6 +36,7 @@ BUILT_SOURCES = parser_bison.h
 lib_LTLIBRARIES = libnftables.la
 
 libnftables_la_SOURCES =			\
+		base64.c			\
 		rule.c				\
 		statement.c			\
 		cache.c				\
@@ -89,7 +90,7 @@ libparser_la_CFLAGS = ${AM_CFLAGS} \
 		      -Wno-undef \
 		      -Wno-redundant-decls
 
-libnftables_la_LIBADD = ${LIBMNL_LIBS} ${LIBNFTNL_LIBS} libparser.la
+libnftables_la_LIBADD = ${LIBMNL_LIBS} ${LIBNFTNL_LIBS} ${Z_LIBS} libparser.la
 libnftables_la_LDFLAGS = -version-info ${libnftables_LIBVERSION} \
 			 -Wl,--version-script=$(srcdir)/libnftables.map
 
diff --git a/src/base64.c b/src/base64.c
new file mode 100644
index 0000000000000..c8abb1e5c3fca
--- /dev/null
+++ b/src/base64.c
@@ -0,0 +1,170 @@
+/*
+ * Base64 encoding/decoding (RFC1341)
+ * Copyright (c) 2005-2011, Jouni Malinen <j@w1.fi>
+ *
+ * This software may be distributed under the terms of the BSD license.
+ * See README for more details.
+ *
+ * Adjustments for nftables:
+ * - headers updated
+ * - dropped code to break long lines
+ */
+
+//#include "includes.h"
+#include <stddef.h>
+#include <string.h>
+#include <utils.h>
+
+//#include "os.h"
+#define os_malloc	xmalloc
+#define os_memset	memset
+#define os_free		free
+
+#include "base64.h"
+
+static const unsigned char base64_table[65] =
+	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
+
+/**
+ * base64_encode - Base64 encode
+ * @src: Data to be encoded
+ * @len: Length of the data to be encoded
+ * @out_len: Pointer to output length variable, or %NULL if not used
+ * Returns: Allocated buffer of out_len bytes of encoded data,
+ * or %NULL on failure
+ *
+ * Caller is responsible for freeing the returned buffer. Returned buffer is
+ * nul terminated to make it easier to use as a C string. The nul terminator is
+ * not included in out_len.
+ */
+unsigned char * base64_encode(const unsigned char *src, size_t len,
+			      size_t *out_len)
+{
+	unsigned char *out, *pos;
+	const unsigned char *end, *in;
+	size_t olen;
+	int line_len;
+
+	olen = len * 4 / 3 + 4; /* 3-byte blocks to 4-byte */
+	olen += olen / 72; /* line feeds */
+	olen++; /* nul termination */
+	if (olen < len)
+		return NULL; /* integer overflow */
+	out = os_malloc(olen);
+	if (out == NULL)
+		return NULL;
+
+	end = src + len;
+	in = src;
+	pos = out;
+	line_len = 0;
+	while (end - in >= 3) {
+		*pos++ = base64_table[in[0] >> 2];
+		*pos++ = base64_table[((in[0] & 0x03) << 4) | (in[1] >> 4)];
+		*pos++ = base64_table[((in[1] & 0x0f) << 2) | (in[2] >> 6)];
+		*pos++ = base64_table[in[2] & 0x3f];
+		in += 3;
+		line_len += 4;
+#if 0
+		if (line_len >= 72) {
+			*pos++ = '\n';
+			line_len = 0;
+		}
+#endif
+	}
+
+	if (end - in) {
+		*pos++ = base64_table[in[0] >> 2];
+		if (end - in == 1) {
+			*pos++ = base64_table[(in[0] & 0x03) << 4];
+			*pos++ = '=';
+		} else {
+			*pos++ = base64_table[((in[0] & 0x03) << 4) |
+					      (in[1] >> 4)];
+			*pos++ = base64_table[(in[1] & 0x0f) << 2];
+		}
+		*pos++ = '=';
+		line_len += 4;
+	}
+
+#if 0
+	if (line_len)
+		*pos++ = '\n';
+#endif
+
+	*pos = '\0';
+	if (out_len)
+		*out_len = pos - out;
+	return out;
+}
+
+
+/**
+ * base64_decode - Base64 decode
+ * @src: Data to be decoded
+ * @len: Length of the data to be decoded
+ * @out_len: Pointer to output length variable
+ * Returns: Allocated buffer of out_len bytes of decoded data,
+ * or %NULL on failure
+ *
+ * Caller is responsible for freeing the returned buffer.
+ */
+unsigned char * base64_decode(const unsigned char *src, size_t len,
+			      size_t *out_len)
+{
+	unsigned char dtable[256], *out, *pos, block[4], tmp;
+	size_t i, count, olen;
+	int pad = 0;
+
+	os_memset(dtable, 0x80, 256);
+	for (i = 0; i < sizeof(base64_table) - 1; i++)
+		dtable[base64_table[i]] = (unsigned char) i;
+	dtable['='] = 0;
+
+	count = 0;
+	for (i = 0; i < len; i++) {
+		if (dtable[src[i]] != 0x80)
+			count++;
+	}
+
+	if (count == 0 || count % 4)
+		return NULL;
+
+	olen = count / 4 * 3;
+	pos = out = os_malloc(olen);
+	if (out == NULL)
+		return NULL;
+
+	count = 0;
+	for (i = 0; i < len; i++) {
+		tmp = dtable[src[i]];
+		if (tmp == 0x80)
+			continue;
+
+		if (src[i] == '=')
+			pad++;
+		block[count] = tmp;
+		count++;
+		if (count == 4) {
+			*pos++ = (block[0] << 2) | (block[1] >> 4);
+			*pos++ = (block[1] << 4) | (block[2] >> 2);
+			*pos++ = (block[2] << 6) | block[3];
+			count = 0;
+			if (pad) {
+				if (pad == 1)
+					pos--;
+				else if (pad == 2)
+					pos -= 2;
+				else {
+					/* Invalid padding */
+					os_free(out);
+					return NULL;
+				}
+				break;
+			}
+		}
+	}
+
+	*out_len = pos - out;
+	return out;
+}
diff --git a/src/evaluate.c b/src/evaluate.c
index 0bf6a0d1b110c..13566ab3de665 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3989,6 +3989,7 @@ int stmt_evaluate(struct eval_ctx *ctx, struct stmt *stmt)
 	case STMT_QUOTA:
 	case STMT_NOTRACK:
 	case STMT_FLOW_OFFLOAD:
+	case STMT_XT:
 		return 0;
 	case STMT_EXPRESSION:
 		return stmt_evaluate_expr(ctx, stmt);
diff --git a/src/json.c b/src/json.c
index 6662f8087736a..1242ab04058bb 100644
--- a/src/json.c
+++ b/src/json.c
@@ -7,6 +7,7 @@
 #include <netlink.h>
 #include <rule.h>
 #include <rt.h>
+#include <xt.h>
 
 #include <netdb.h>
 #include <netinet/icmp6.h>
@@ -82,12 +83,6 @@ static json_t *stmt_print_json(const struct stmt *stmt, struct output_ctx *octx)
 	char buf[1024];
 	FILE *fp;
 
-	/* XXX: Can't be supported at this point:
-	 * xt_stmt_xlate() ignores output_fp.
-	 */
-	if (stmt->ops->type == STMT_XT)
-		return json_pack("{s:n}", "xt");
-
 	if (stmt->ops->json)
 		return stmt->ops->json(stmt, octx);
 
@@ -1624,6 +1619,24 @@ json_t *optstrip_stmt_json(const struct stmt *stmt, struct output_ctx *octx)
 			 expr_print_json(stmt->optstrip.expr, octx));
 }
 
+json_t *xt_stmt_json(const struct stmt *stmt, struct output_ctx *octx)
+{
+	static const char *xt_typename[] = {
+		[NFT_XT_MATCH]		= "match",
+		[NFT_XT_TARGET]		= "target",
+		[NFT_XT_WATCHER]	= "watcher",
+		[NFT_XT_MAX]		= "unknown"
+	};
+	unsigned char *b64_buf;
+	json_t *json;
+
+	b64_buf = xt_stmt_blob_encode(stmt);
+	json = json_pack("[s, s, s]", xt_typename[stmt->xt.type],
+			 stmt->xt.name, b64_buf);
+	xfree(b64_buf);
+	return json;
+}
+
 static json_t *table_print_json_full(struct netlink_ctx *ctx,
 				     struct table *table)
 {
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index c8bbcb7452b05..d372919662578 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -20,6 +20,7 @@
 #include <gmputil.h>
 #include <utils.h>
 #include <netinet/in.h>
+#include <zlib.h>
 
 #include <linux/netfilter.h>
 #include <libnftnl/udata.h>
@@ -1365,6 +1366,35 @@ static void netlink_gen_optstrip_stmt(struct netlink_linearize_ctx *ctx,
 	nft_rule_add_expr(ctx, nle, &expr->location);
 }
 
+static void netlink_gen_xt_stmt(struct netlink_linearize_ctx *ctx,
+				const struct stmt *stmt)
+{
+	void *data = xmalloc(stmt->xt.infolen);
+	struct nftnl_expr *nle;
+
+	memcpy(data, stmt->xt.info, stmt->xt.infolen);
+
+	switch(stmt->xt.type) {
+	case NFT_XT_MATCH:
+		nle = alloc_nft_expr("match");
+		nftnl_expr_set_str(nle, NFTNL_EXPR_MT_NAME, stmt->xt.name);
+		nftnl_expr_set_u32(nle, NFTNL_EXPR_MT_REV, stmt->xt.rev);
+		nftnl_expr_set(nle, NFTNL_EXPR_MT_INFO, data, stmt->xt.infolen);
+		break;
+	case NFT_XT_TARGET:
+	case NFT_XT_WATCHER:
+		nle = alloc_nft_expr("target");
+		nftnl_expr_set_str(nle, NFTNL_EXPR_TG_NAME, stmt->xt.name);
+		nftnl_expr_set_u32(nle, NFTNL_EXPR_TG_REV, stmt->xt.rev);
+		nftnl_expr_set(nle, NFTNL_EXPR_TG_INFO, data, stmt->xt.infolen);
+		break;
+	default:
+		free(data);
+		return;
+	}
+	nft_rule_add_expr(ctx, nle, &stmt->location);
+}
+
 static void netlink_gen_queue_stmt(struct netlink_linearize_ctx *ctx,
 				 const struct stmt *stmt)
 {
@@ -1630,6 +1660,8 @@ static void netlink_gen_stmt(struct netlink_linearize_ctx *ctx,
 		return netlink_gen_chain_stmt(ctx, stmt);
 	case STMT_OPTSTRIP:
 		return netlink_gen_optstrip_stmt(ctx, stmt);
+	case STMT_XT:
+		return netlink_gen_xt_stmt(ctx, stmt);
 	default:
 		BUG("unknown statement type %s\n", stmt->ops->name);
 	}
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 760c23cf33223..c23d0178b2fab 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -626,6 +626,13 @@ int nft_lex(void *, void *, void *);
 %token IN			"in"
 %token OUT			"out"
 
+%token XT			"xt"
+%token MATCH			"match"
+%token TARGET			"target"
+%token WATCHER			"watcher"
+%type <stmt>			xt_stmt
+%type <val>			xt_stmt_type
+
 %type <limit_rate>		limit_rate_pkts
 %type <limit_rate>		limit_rate_bytes
 
@@ -991,6 +998,7 @@ close_scope_udplite	: { scanner_pop_start_cond(nft->scanner, PARSER_SC_EXPR_UDPL
 
 close_scope_log		: { scanner_pop_start_cond(nft->scanner, PARSER_SC_STMT_LOG); }
 close_scope_synproxy	: { scanner_pop_start_cond(nft->scanner, PARSER_SC_STMT_SYNPROXY); }
+close_scope_xt		: { scanner_pop_start_cond(nft->scanner, PARSER_SC_STMT_XT); }
 
 common_block		:	INCLUDE		QUOTED_STRING	stmt_separator
 			{
@@ -2879,6 +2887,26 @@ stmt			:	verdict_stmt
 			|	synproxy_stmt	close_scope_synproxy
 			|	chain_stmt
 			|	optstrip_stmt
+			|	xt_stmt
+			;
+
+xt_stmt_type		:	MATCH	{ $$ = NFT_XT_MATCH; }
+			|	TARGET	{ $$ = NFT_XT_TARGET; }
+			|	WATCHER	{ $$ = NFT_XT_WATCHER; }
+			;
+
+xt_stmt			:	XT	xt_stmt_type	STRING	STRING	close_scope_xt
+			{
+				$$ = xt_stmt_alloc(&@$);
+				$$->xt.type = $2;
+				$$->xt.name = $3;
+				if (xt_stmt_blob_decode($$, $4, &@4, state->msgs)) {
+					xfree($$);
+					xfree($4);
+					YYERROR;
+				}
+				xfree($4);
+			}
 			;
 
 chain_stmt_type		:	JUMP	{ $$ = NFT_JUMP; }
diff --git a/src/parser_json.c b/src/parser_json.c
index 76c268f857202..c4e4ef4f9e1f6 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -13,6 +13,7 @@
 #include <rule.h>
 #include <sctp_chunk.h>
 #include <socket.h>
+#include <xt.h>
 
 #include <netdb.h>
 #include <netinet/icmp6.h>
@@ -2707,6 +2708,40 @@ static struct stmt *json_parse_optstrip_stmt(struct json_ctx *ctx,
 	return expr ? optstrip_stmt_alloc(int_loc, expr) : NULL;
 }
 
+static enum nft_xt_type xt_stmt_typeval(const char *type)
+{
+	if (!strcmp(type, "match"))
+		return NFT_XT_MATCH;
+	if (!strcmp(type, "target"))
+		return NFT_XT_TARGET;
+	if (!strcmp(type, "watcher"))
+		return NFT_XT_WATCHER;
+	return NFT_XT_MAX;
+}
+
+static struct stmt *json_parse_xt_stmt(struct json_ctx *ctx,
+				       const char *key, json_t *value)
+{
+	char *type, *name, *blob;
+	struct stmt *stmt;
+
+	if (json_unpack_err(ctx, value, "[s, s, s]", &type, &name, &blob))
+		return NULL;
+
+	stmt = xt_stmt_alloc(int_loc);
+	stmt->xt.type = xt_stmt_typeval(type);
+	if (stmt->xt.type == NFT_XT_MAX) {
+		json_error(ctx, "Invalid xt type name '%s'.", type);
+		return NULL;
+	}
+	stmt->xt.name = xstrdup(name);
+	if (xt_stmt_blob_decode(stmt, blob, int_loc, ctx->msgs)) {
+		json_error(ctx, "Invalid base64 data in '%s'.", blob);
+		return NULL;
+	}
+	return stmt;
+}
+
 static struct stmt *json_parse_stmt(struct json_ctx *ctx, json_t *root)
 {
 	struct {
@@ -2745,6 +2780,7 @@ static struct stmt *json_parse_stmt(struct json_ctx *ctx, json_t *root)
 		{ "synproxy", json_parse_synproxy_stmt },
 		{ "reset", json_parse_optstrip_stmt },
 		{ "secmark", json_parse_secmark_stmt },
+		{ "xt", json_parse_xt_stmt },
 	};
 	const char *type;
 	unsigned int i;
diff --git a/src/scanner.l b/src/scanner.l
index 1371cd044b65a..e933d32f13d1a 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -125,6 +125,7 @@ quotedstring	\"[^"]*\"
 asteriskstring	({string}\*|{string}\\\*|\\\*|{string}\\\*{string})
 comment		#.*$
 slash		\/
+base64string	({letter}|{digit}|\+|{slash})*=*
 
 timestring	([0-9]+d)?([0-9]+h)?([0-9]+m)?([0-9]+s)?([0-9]+ms)?
 
@@ -247,6 +248,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 %s SCANSTATE_STMT_REJECT
 %s SCANSTATE_STMT_SYNPROXY
 %s SCANSTATE_STMT_TPROXY
+%s SCANSTATE_STMT_XT
 
 %%
 
@@ -411,6 +413,13 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 	"group"			{ return GROUP; }
 }
 
+"xt"			{ scanner_push_start_cond(yyscanner, SCANSTATE_STMT_XT); return XT; }
+<SCANSTATE_STMT_XT>{
+	"match"		{ return MATCH; }
+	"target"	{ return TARGET; }
+	"watcher"	{ return WATCHER; }
+}
+
 "queue"			{ scanner_push_start_cond(yyscanner, SCANSTATE_EXPR_QUEUE); return QUEUE;}
 <SCANSTATE_EXPR_QUEUE>{
 	"num"		{ return QUEUENUM;}
@@ -846,6 +855,11 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 				return STRING;
 			}
 
+{base64string}		{
+				yylval->string = xstrdup(yytext);
+				return STRING;
+			}
+
 \\{newline}		{
 				reset_pos(yyget_extra(yyscanner), yylloc);
 			}
diff --git a/src/statement.c b/src/statement.c
index 327d00f99200a..eafc51c484de9 100644
--- a/src/statement.c
+++ b/src/statement.c
@@ -997,6 +997,7 @@ static const struct stmt_ops xt_stmt_ops = {
 	.name		= "xt",
 	.print		= xt_stmt_print,
 	.destroy	= xt_stmt_destroy,
+	.json		= xt_stmt_json,
 };
 
 struct stmt *xt_stmt_alloc(const struct location *loc)
diff --git a/src/xt.c b/src/xt.c
index f14f40157ba10..9a326fd313233 100644
--- a/src/xt.c
+++ b/src/xt.c
@@ -17,6 +17,7 @@
 #include <netlink.h>
 #include <xt.h>
 #include <erec.h>
+#include <base64.h>
 
 #include <libmnl/libmnl.h>
 #include <linux/netfilter/nfnetlink.h>
@@ -31,9 +32,87 @@
 
 static void *xt_entry_alloc(const struct xt_stmt *xt, uint32_t af);
 #endif
+#ifdef HAVE_ZLIB
+#include <zlib.h>
+#endif
+
+struct xt_stmt_blob {
+	uint32_t	orig_len;
+	uint8_t		rev;
+	unsigned char	data[];
+};
+
+unsigned char *xt_stmt_blob_encode(const struct stmt *stmt)
+{
+#ifdef HAVE_ZLIB
+	size_t complen = compressBound(stmt->xt.infolen);
+#else
+	size_t complen = stmt->xt.infolen;
+#endif
+	struct xt_stmt_blob *blob;
+	unsigned char *b64_buf;
+
+	blob = xzalloc(sizeof(*blob) + complen);
+	blob->rev = stmt->xt.rev;
+	blob->orig_len = stmt->xt.infolen;
+#ifdef HAVE_ZLIB
+	compress(blob->data, &complen,
+		 (const Bytef *)stmt->xt.info, stmt->xt.infolen);
+#else
+	memcpy(blob->data, stmt->xt.info, stmt->xt.infolen);
+#endif
+
+	b64_buf = base64_encode((const unsigned char *)blob,
+				complen + sizeof(*blob), NULL);
+	xfree(blob);
+	return b64_buf;
+}
+
+int xt_stmt_blob_decode(struct stmt *stmt, const char *b64_string,
+			const struct location *b64_loc, struct list_head *msgs)
+{
+	struct xt_stmt_blob *blob;
+	size_t bloblen;
+	int ret = 0;
+
+	blob = (void *)base64_decode((const unsigned char *)b64_string,
+				     strlen(b64_string), &bloblen);
+	if (!blob) {
+		erec_queue(error(b64_loc, "invalid base64 string"), msgs);
+		return 1;
+	}
+
+	bloblen -= sizeof(*blob);
+
+	stmt->xt.rev = blob->rev;
+	stmt->xt.info = xmalloc(blob->orig_len);
+	stmt->xt.infolen = blob->orig_len;
+	if (blob->orig_len == bloblen) {
+		memcpy(stmt->xt.info, blob->data, stmt->xt.infolen);
+#ifdef HAVE_ZLIB
+	} else if (uncompress(stmt->xt.info, &stmt->xt.infolen,
+			      blob->data, bloblen) != Z_OK) {
+		erec_queue(error(b64_loc, "blob decompression failed"), msgs);
+#else
+	} else {
+		erec_queue(error(b64_loc,
+				 "compressed blobs not supported"), msgs);
+#endif
+		ret = 1;
+	}
+	free(blob);
+	return ret;
+}
 
 void xt_stmt_xlate(const struct stmt *stmt, struct output_ctx *octx)
 {
+	static const char *xt_typename[] = {
+		[NFT_XT_MATCH]		= "match",
+		[NFT_XT_TARGET]		= "target",
+		[NFT_XT_WATCHER]	= "watcher",
+		[NFT_XT_MAX]		= "unknown"
+	};
+	unsigned char *b64_buf;
 #ifdef HAVE_LIBXTABLES
 	struct xt_xlate *xl = xt_xlate_alloc(10240);
 	struct xtables_target *tg;
@@ -71,9 +150,10 @@ void xt_stmt_xlate(const struct stmt *stmt, struct output_ctx *octx)
 
 			mt->xlate(xl, &params);
 			nft_print(octx, "%s", xt_xlate_get(xl));
-		} else if (mt->print) {
-			printf("#");
-			mt->print(&entry, m, 0);
+			xfree(m);
+			xfree(entry);
+			xt_xlate_free(xl);
+			return;
 		}
 		xfree(m);
 		break;
@@ -104,9 +184,10 @@ void xt_stmt_xlate(const struct stmt *stmt, struct output_ctx *octx)
 
 			tg->xlate(xl, &params);
 			nft_print(octx, "%s", xt_xlate_get(xl));
-		} else if (tg->print) {
-			printf("#");
-			tg->print(NULL, t, 0);
+			xfree(t);
+			xfree(entry);
+			xt_xlate_free(xl);
+			return;
 		}
 		xfree(t);
 		break;
@@ -116,9 +197,11 @@ void xt_stmt_xlate(const struct stmt *stmt, struct output_ctx *octx)
 
 	xt_xlate_free(xl);
 	xfree(entry);
-#else
-	nft_print(octx, "# xt_%s", stmt->xt.name);
 #endif
+	b64_buf = xt_stmt_blob_encode(stmt);
+	nft_print(octx, "xt %s %s %s",
+		  xt_typename[stmt->xt.type], stmt->xt.name, b64_buf);
+	xfree(b64_buf);
 }
 
 void xt_stmt_destroy(struct stmt *stmt)
-- 
2.38.0


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

* [nft PATCH v2 3/4] xt: Put match/target translation into own functions
  2022-11-17 17:45 [nft PATCH v2 0/4] xt: Implement dump and restore support Phil Sutter
  2022-11-17 17:45 ` [nft PATCH v2 1/4] xt: Delay libxtables access until translation Phil Sutter
  2022-11-17 17:45 ` [nft PATCH v2 2/4] xt: Implement dump and restore support Phil Sutter
@ 2022-11-17 17:45 ` Phil Sutter
  2022-11-17 17:45 ` [nft PATCH v2 4/4] xt: Detect xlate callback failure Phil Sutter
  2022-11-17 21:13 ` [nft PATCH v2 0/4] xt: Implement dump and restore support Florian Westphal
  4 siblings, 0 replies; 18+ messages in thread
From: Phil Sutter @ 2022-11-17 17:45 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Reduce the size and indenting level of xt_stmt_xlate() a bit, also fix
for error printing to stderr irrespective of octx->error_fp value.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Change fprintf() calls to respect octx.
---
 src/xt.c | 144 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 78 insertions(+), 66 deletions(-)

diff --git a/src/xt.c b/src/xt.c
index 9a326fd313233..e3063612c353e 100644
--- a/src/xt.c
+++ b/src/xt.c
@@ -104,6 +104,75 @@ int xt_stmt_blob_decode(struct stmt *stmt, const char *b64_string,
 	return ret;
 }
 
+#ifdef HAVE_LIBXTABLES
+static bool xt_stmt_xlate_match(const struct stmt *stmt, void *entry,
+				struct xt_xlate *xl, struct output_ctx *octx)
+{
+	size_t size = XT_ALIGN(sizeof(struct xt_entry_match))
+			+ stmt->xt.infolen;
+	struct xt_xlate_mt_params params = {
+		.ip		= entry,
+		.numeric        = 1,
+	};
+	struct xtables_match *mt;
+	struct xt_entry_match *m;
+
+	mt = xtables_find_match(stmt->xt.name, XTF_TRY_LOAD, NULL);
+	if (!mt) {
+		fprintf(octx->error_fp,
+			"XT match %s not found\n", stmt->xt.name);
+		return false;
+	}
+	if (!mt->xlate)
+		return false;
+
+	m = xzalloc(size);
+	m->u.match_size = size;
+	m->u.user.revision = stmt->xt.rev;
+	memcpy(&m->data, stmt->xt.info, stmt->xt.infolen);
+
+	params.match = m;
+	mt->xlate(xl, &params);
+
+	xfree(m);
+	return true;
+}
+
+static bool xt_stmt_xlate_target(const struct stmt *stmt, void *entry,
+				 struct xt_xlate *xl, struct output_ctx *octx)
+{
+	size_t size = XT_ALIGN(sizeof(struct xt_entry_target))
+			+ stmt->xt.infolen;
+	struct xt_xlate_tg_params params = {
+		.ip		= entry,
+		.numeric        = 1,
+	};
+	struct xtables_target *tg;
+	struct xt_entry_target *t;
+
+	tg = xtables_find_target(stmt->xt.name, XTF_TRY_LOAD);
+	if (!tg) {
+		fprintf(octx->error_fp,
+			"XT target %s not found\n", stmt->xt.name);
+		return false;
+	}
+	if (!tg->xlate)
+		return false;
+
+	t = xzalloc(size);
+	t->u.target_size = size;
+	t->u.user.revision = stmt->xt.rev;
+	memcpy(&t->data, stmt->xt.info, stmt->xt.infolen);
+	strcpy(t->u.user.name, tg->name);
+
+	params.target = t;
+	tg->xlate(xl, &params);
+
+	xfree(t);
+	return true;
+}
+#endif
+
 void xt_stmt_xlate(const struct stmt *stmt, struct output_ctx *octx)
 {
 	static const char *xt_typename[] = {
@@ -115,11 +184,7 @@ void xt_stmt_xlate(const struct stmt *stmt, struct output_ctx *octx)
 	unsigned char *b64_buf;
 #ifdef HAVE_LIBXTABLES
 	struct xt_xlate *xl = xt_xlate_alloc(10240);
-	struct xtables_target *tg;
-	struct xt_entry_target *t;
-	struct xtables_match *mt;
-	struct xt_entry_match *m;
-	size_t size;
+	bool xlated = false;
 	void *entry;
 
 	xtables_set_nfproto(stmt->xt.family);
@@ -127,76 +192,23 @@ void xt_stmt_xlate(const struct stmt *stmt, struct output_ctx *octx)
 
 	switch (stmt->xt.type) {
 	case NFT_XT_MATCH:
-		mt = xtables_find_match(stmt->xt.name, XTF_TRY_LOAD, NULL);
-		if (!mt) {
-			fprintf(stderr, "XT match %s not found\n",
-				stmt->xt.name);
-			return;
-		}
-		size = XT_ALIGN(sizeof(*m)) + stmt->xt.infolen;
-
-		m = xzalloc(size);
-		memcpy(&m->data, stmt->xt.info, stmt->xt.infolen);
-
-		m->u.match_size = size;
-		m->u.user.revision = stmt->xt.rev;
-
-		if (mt->xlate) {
-			struct xt_xlate_mt_params params = {
-				.ip		= entry,
-				.match		= m,
-				.numeric        = 1,
-			};
-
-			mt->xlate(xl, &params);
-			nft_print(octx, "%s", xt_xlate_get(xl));
-			xfree(m);
-			xfree(entry);
-			xt_xlate_free(xl);
-			return;
-		}
-		xfree(m);
+		xlated = xt_stmt_xlate_match(stmt, entry, xl, octx);
 		break;
 	case NFT_XT_WATCHER:
 	case NFT_XT_TARGET:
-		tg = xtables_find_target(stmt->xt.name, XTF_TRY_LOAD);
-		if (!tg) {
-			fprintf(stderr, "XT target %s not found\n",
-				stmt->xt.name);
-			return;
-		}
-		size = XT_ALIGN(sizeof(*t)) + stmt->xt.infolen;
-
-		t = xzalloc(size);
-		memcpy(&t->data, stmt->xt.info, stmt->xt.infolen);
-
-		t->u.target_size = size;
-		t->u.user.revision = stmt->xt.rev;
-
-		strcpy(t->u.user.name, tg->name);
-
-		if (tg->xlate) {
-			struct xt_xlate_tg_params params = {
-				.ip		= entry,
-				.target		= t,
-				.numeric        = 1,
-			};
-
-			tg->xlate(xl, &params);
-			nft_print(octx, "%s", xt_xlate_get(xl));
-			xfree(t);
-			xfree(entry);
-			xt_xlate_free(xl);
-			return;
-		}
-		xfree(t);
+		xlated = xt_stmt_xlate_target(stmt, entry, xl, octx);
 		break;
 	default:
 		break;
 	}
 
-	xt_xlate_free(xl);
 	xfree(entry);
+	if (xlated) {
+		nft_print(octx, "%s", xt_xlate_get(xl));
+		xt_xlate_free(xl);
+		return;
+	}
+	xt_xlate_free(xl);
 #endif
 	b64_buf = xt_stmt_blob_encode(stmt);
 	nft_print(octx, "xt %s %s %s",
-- 
2.38.0


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

* [nft PATCH v2 4/4] xt: Detect xlate callback failure
  2022-11-17 17:45 [nft PATCH v2 0/4] xt: Implement dump and restore support Phil Sutter
                   ` (2 preceding siblings ...)
  2022-11-17 17:45 ` [nft PATCH v2 3/4] xt: Put match/target translation into own functions Phil Sutter
@ 2022-11-17 17:45 ` Phil Sutter
  2022-11-17 21:13 ` [nft PATCH v2 0/4] xt: Implement dump and restore support Florian Westphal
  4 siblings, 0 replies; 18+ messages in thread
From: Phil Sutter @ 2022-11-17 17:45 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

If an extension's xlate callback returns 0, translation is at least
incomplete. Discard the result and resort to opaque dump format in this
case.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/xt.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/xt.c b/src/xt.c
index e3063612c353e..178761a42018d 100644
--- a/src/xt.c
+++ b/src/xt.c
@@ -116,6 +116,7 @@ static bool xt_stmt_xlate_match(const struct stmt *stmt, void *entry,
 	};
 	struct xtables_match *mt;
 	struct xt_entry_match *m;
+	int rc;
 
 	mt = xtables_find_match(stmt->xt.name, XTF_TRY_LOAD, NULL);
 	if (!mt) {
@@ -132,10 +133,10 @@ static bool xt_stmt_xlate_match(const struct stmt *stmt, void *entry,
 	memcpy(&m->data, stmt->xt.info, stmt->xt.infolen);
 
 	params.match = m;
-	mt->xlate(xl, &params);
+	rc = mt->xlate(xl, &params);
 
 	xfree(m);
-	return true;
+	return rc != 0;
 }
 
 static bool xt_stmt_xlate_target(const struct stmt *stmt, void *entry,
@@ -149,6 +150,7 @@ static bool xt_stmt_xlate_target(const struct stmt *stmt, void *entry,
 	};
 	struct xtables_target *tg;
 	struct xt_entry_target *t;
+	int rc;
 
 	tg = xtables_find_target(stmt->xt.name, XTF_TRY_LOAD);
 	if (!tg) {
@@ -166,10 +168,10 @@ static bool xt_stmt_xlate_target(const struct stmt *stmt, void *entry,
 	strcpy(t->u.user.name, tg->name);
 
 	params.target = t;
-	tg->xlate(xl, &params);
+	rc = tg->xlate(xl, &params);
 
 	xfree(t);
-	return true;
+	return rc != 0;
 }
 #endif
 
-- 
2.38.0


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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-17 17:45 [nft PATCH v2 0/4] xt: Implement dump and restore support Phil Sutter
                   ` (3 preceding siblings ...)
  2022-11-17 17:45 ` [nft PATCH v2 4/4] xt: Detect xlate callback failure Phil Sutter
@ 2022-11-17 21:13 ` Florian Westphal
  2022-11-18  9:40   ` Pablo Neira Ayuso
  2022-11-18  9:47   ` Phil Sutter
  4 siblings, 2 replies; 18+ messages in thread
From: Florian Westphal @ 2022-11-17 21:13 UTC (permalink / raw)
  To: Phil Sutter; +Cc: Pablo Neira Ayuso, netfilter-devel

Phil Sutter <phil@nwl.cc> wrote:
> If nft can't translate a compat expression, dump it in a format that can
> be restored later without losing data, thereby keeping the ruleset
> intact.

Why? :-( This cements nft_compat.c forever.

If we're goping to do it lets at least dump it properly,
i.e.  nft ... add rule compat "-m conntrack --ctstate NEW".

At this time I'd rather like a time machine to prevent nft_compat.c from
getting merged :-(

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-17 21:13 ` [nft PATCH v2 0/4] xt: Implement dump and restore support Florian Westphal
@ 2022-11-18  9:40   ` Pablo Neira Ayuso
  2022-11-18  9:55     ` Phil Sutter
  2022-11-18  9:47   ` Phil Sutter
  1 sibling, 1 reply; 18+ messages in thread
From: Pablo Neira Ayuso @ 2022-11-18  9:40 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Phil Sutter, netfilter-devel

On Thu, Nov 17, 2022 at 10:13:47PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > If nft can't translate a compat expression, dump it in a format that can
> > be restored later without losing data, thereby keeping the ruleset
> > intact.
> 
> Why? :-( This cements nft_compat.c forever.
>
> If we're goping to do it lets at least dump it properly,
> i.e.  nft ... add rule compat "-m conntrack --ctstate NEW".

We might have to support this mixed syntax forever.  I proposed this
long long time ago, when nftables was supporting ~25% of the iptables
feature-set. These days, where they are almost on par (actually
nftables being a lot more expressive than iptables), I am not sure it
makes sense to follow this path anymore.

Unless you refer to dumping a listing which nft cannot load as a way
to provide a listing that is comprehensible, but that cannot be loaded
by the user.

> At this time I'd rather like a time machine to prevent nft_compat.c from
> getting merged :-(

I agree we should have added native translations for iptables-nft
sooner, but it was never a priority for anyone so far.

This "forward compatibility" issue (pretending old tool versions can
interpret new revisions / features loaded by newer tool versions) we
are trying to deal is hard, we already discussed none of the other
existing tooling (ethtool, iproute2, etc.) supports for this.

If you prefer to go for the _USERDATA area as a last resort, I'm OK
with it, this requires no kernel patches, and it will be used only for
the "forward compatibility" scenario (last resort)

We can also resort on displaying the raw expressions, so the user gets
a meaningful output that cannot be loaded again.

I think this more or less a summary of what we discussed in the NFWS.

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-17 21:13 ` [nft PATCH v2 0/4] xt: Implement dump and restore support Florian Westphal
  2022-11-18  9:40   ` Pablo Neira Ayuso
@ 2022-11-18  9:47   ` Phil Sutter
  2022-11-18 10:11     ` Pablo Neira Ayuso
  1 sibling, 1 reply; 18+ messages in thread
From: Phil Sutter @ 2022-11-18  9:47 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Pablo Neira Ayuso, netfilter-devel

On Thu, Nov 17, 2022 at 10:13:47PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > If nft can't translate a compat expression, dump it in a format that can
> > be restored later without losing data, thereby keeping the ruleset
> > intact.
> 
> Why? :-( This cements nft_compat.c forever.

To avoid silently breaking a ruleset. It is a last resort measure for
cases where nft can't translate the ruleset into something meaningful. I
already submitted a patch adding a warning when listing tables
containing compat expressions (status unclear), but a real alternative
to the above would be to abort the listing or to ignore the table.

Listing the ruleset in translated form when iptables-nft can't parse the
translation or without translation but still maintaining syntax to be
parsed without error during a later restore is almost luring users into
doing stupid things.

> If we're goping to do it lets at least dump it properly,
> i.e.  nft ... add rule compat "-m conntrack --ctstate NEW".

This will make things worse: People will understand the format and start
using it despite the warnings. This adds a new user base to compat
expressions. I don't dare claiming nobody would start crafting compat
expressions in my zip-dump format, but it's at least a larger obstacle.
:D

> At this time I'd rather like a time machine to prevent nft_compat.c from
> getting merged :-(

If you do, please convince Pablo to not push iptables commit 384958620a.
I think it opened the can of worms we're trying to confine here.

Cheers, Phil

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-18  9:40   ` Pablo Neira Ayuso
@ 2022-11-18  9:55     ` Phil Sutter
  0 siblings, 0 replies; 18+ messages in thread
From: Phil Sutter @ 2022-11-18  9:55 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

On Fri, Nov 18, 2022 at 10:40:30AM +0100, Pablo Neira Ayuso wrote:
> On Thu, Nov 17, 2022 at 10:13:47PM +0100, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > If nft can't translate a compat expression, dump it in a format that can
> > > be restored later without losing data, thereby keeping the ruleset
> > > intact.
> > 
> > Why? :-( This cements nft_compat.c forever.
> >
> > If we're goping to do it lets at least dump it properly,
> > i.e.  nft ... add rule compat "-m conntrack --ctstate NEW".
> 
> We might have to support this mixed syntax forever.  I proposed this
> long long time ago, when nftables was supporting ~25% of the iptables
> feature-set. These days, where they are almost on par (actually
> nftables being a lot more expressive than iptables), I am not sure it
> makes sense to follow this path anymore.
> 
> Unless you refer to dumping a listing which nft cannot load as a way
> to provide a listing that is comprehensible, but that cannot be loaded
> by the user.

Or a big fat warning, but I fear anything that allows
| nft list ruleset | nft -f -
will simply be ignored until the problems become obvious.

> > At this time I'd rather like a time machine to prevent nft_compat.c from
> > getting merged :-(
> 
> I agree we should have added native translations for iptables-nft
> sooner, but it was never a priority for anyone so far.
> 
> This "forward compatibility" issue (pretending old tool versions can
> interpret new revisions / features loaded by newer tool versions) we
> are trying to deal is hard, we already discussed none of the other
> existing tooling (ethtool, iproute2, etc.) supports for this.
> 
> If you prefer to go for the _USERDATA area as a last resort, I'm OK
> with it, this requires no kernel patches, and it will be used only for
> the "forward compatibility" scenario (last resort)
> 
> We can also resort on displaying the raw expressions, so the user gets
> a meaningful output that cannot be loaded again.
> 
> I think this more or less a summary of what we discussed in the NFWS.

Pablo, I think you're mixing up two things here:

This "support dump and load of compat expression" feature is to sanitize
the current situation with up to date iptables and nftables.

The forward compat (you were right about the direction) issue is mostly
about iptables-nft vs. itself in an older version.

Cheers, Phil

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-18  9:47   ` Phil Sutter
@ 2022-11-18 10:11     ` Pablo Neira Ayuso
  2022-11-18 10:42       ` Phil Sutter
  0 siblings, 1 reply; 18+ messages in thread
From: Pablo Neira Ayuso @ 2022-11-18 10:11 UTC (permalink / raw)
  To: Phil Sutter, Florian Westphal, netfilter-devel

Merging threads.

On Fri, Nov 18, 2022 at 10:55:04AM +0100, Phil Sutter wrote:
[...]
> > I think this more or less a summary of what we discussed in the NFWS.
>
> Pablo, I think you're mixing up two things here:
>
> This "support dump and load of compat expression" feature is to sanitize
> the current situation with up to date iptables and nftables.

OK, then the problem we discuss is mixing iptables-nft and nftables.

On Fri, Nov 18, 2022 at 10:47:48AM +0100, Phil Sutter wrote:
[...]
> > At this time I'd rather like a time machine to prevent nft_compat.c from
> > getting merged :-(
>
> If you do, please convince Pablo to not push iptables commit 384958620a.
> I think it opened the can of worms we're trying to confine here.

It could be worst, if iptables-nft would not be in place, then old
iptables-legacy and new nftables rules would have no visibility each
other.

With iptables-nft we have a way to move forward:

- Replace nft_compat by native expressions from iptables-nft.
- Extend iptables-nft to understand more complex expressions, worst
  case dump a native representation.

Why don't we just move ahead this path instead of spinning around the
compat layer? This only requires userspace updates on iptables-nft.

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-18 10:11     ` Pablo Neira Ayuso
@ 2022-11-18 10:42       ` Phil Sutter
  2022-11-18 11:46         ` Florian Westphal
  0 siblings, 1 reply; 18+ messages in thread
From: Phil Sutter @ 2022-11-18 10:42 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

On Fri, Nov 18, 2022 at 11:11:42AM +0100, Pablo Neira Ayuso wrote:
> Merging threads.
> 
> On Fri, Nov 18, 2022 at 10:55:04AM +0100, Phil Sutter wrote:
> [...]
> > > I think this more or less a summary of what we discussed in the NFWS.
> >
> > Pablo, I think you're mixing up two things here:
> >
> > This "support dump and load of compat expression" feature is to sanitize
> > the current situation with up to date iptables and nftables.
> 
> OK, then the problem we discuss is mixing iptables-nft and nftables.
> 
> On Fri, Nov 18, 2022 at 10:47:48AM +0100, Phil Sutter wrote:
> [...]
> > > At this time I'd rather like a time machine to prevent nft_compat.c from
> > > getting merged :-(
> >
> > If you do, please convince Pablo to not push iptables commit 384958620a.
> > I think it opened the can of worms we're trying to confine here.
> 
> It could be worst, if iptables-nft would not be in place, then old
> iptables-legacy and new nftables rules would have no visibility each
> other.
> 
> With iptables-nft we have a way to move forward:
> 
> - Replace nft_compat by native expressions from iptables-nft.
> - Extend iptables-nft to understand more complex expressions, worst
>   case dump a native representation.
> 
> Why don't we just move ahead this path instead of spinning around the
> compat layer? This only requires userspace updates on iptables-nft.

Sure! I'm just picking low hanging fruits first. With even translation
support being still incomplete, I fear it will take a while until the
tools are fluent enough for this to not matter anymore. And then there's
still nftables without libxtables support.

Cheers, Phil

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-18 10:42       ` Phil Sutter
@ 2022-11-18 11:46         ` Florian Westphal
  2022-11-18 12:12           ` Phil Sutter
  0 siblings, 1 reply; 18+ messages in thread
From: Florian Westphal @ 2022-11-18 11:46 UTC (permalink / raw)
  To: Phil Sutter, Pablo Neira Ayuso, Florian Westphal, netfilter-devel

Phil Sutter <phil@nwl.cc> wrote:
> On Fri, Nov 18, 2022 at 11:11:42AM +0100, Pablo Neira Ayuso wrote:
> > Merging threads.
> > 
> > On Fri, Nov 18, 2022 at 10:55:04AM +0100, Phil Sutter wrote:
> > [...]
> > > > I think this more or less a summary of what we discussed in the NFWS.
> > >
> > > Pablo, I think you're mixing up two things here:
> > >
> > > This "support dump and load of compat expression" feature is to sanitize
> > > the current situation with up to date iptables and nftables.
> > 
> > OK, then the problem we discuss is mixing iptables-nft and nftables.
> > 
> > On Fri, Nov 18, 2022 at 10:47:48AM +0100, Phil Sutter wrote:
> > [...]
> > > > At this time I'd rather like a time machine to prevent nft_compat.c from
> > > > getting merged :-(
> > >
> > > If you do, please convince Pablo to not push iptables commit 384958620a.
> > > I think it opened the can of worms we're trying to confine here.
> > 
> > It could be worst, if iptables-nft would not be in place, then old
> > iptables-legacy and new nftables rules would have no visibility each
> > other.
> > 
> > With iptables-nft we have a way to move forward:
> > 
> > - Replace nft_compat by native expressions from iptables-nft.
> > - Extend iptables-nft to understand more complex expressions, worst
> >   case dump a native representation.
> > 
> > Why don't we just move ahead this path instead of spinning around the
> > compat layer? This only requires userspace updates on iptables-nft.
> 
> Sure! I'm just picking low hanging fruits first. With even translation
> support being still incomplete, I fear it will take a while until the
> tools are fluent enough for this to not matter anymore. And then there's
> still nftables without libxtables support.

Then perhaps its better to do following path:
1. Try ->xlate(), if that fails, then print a 'breaking' format?

As far as I understand the problem is the "# comment" - type syntax that
makes nft just skip the incomplete rule, so perhaps just use invalid
format?

Example:

counter packets 0 bytes 0 # name foo interval 250.0ms ewmalog 500.0ms
Instead make this something like
counter packets 0 bytes 0 nft_compat [ RATEEST name foo interval 250.0ms ewmalog 500.0ms ] # unsupported iptables-nft rule

?

I'd like to avoid exposure in the frontend with compatible-restore-approach if possible.

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-18 11:46         ` Florian Westphal
@ 2022-11-18 12:12           ` Phil Sutter
  2022-11-18 12:18             ` Pablo Neira Ayuso
  0 siblings, 1 reply; 18+ messages in thread
From: Phil Sutter @ 2022-11-18 12:12 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Pablo Neira Ayuso, netfilter-devel

On Fri, Nov 18, 2022 at 12:46:43PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > On Fri, Nov 18, 2022 at 11:11:42AM +0100, Pablo Neira Ayuso wrote:
> > > Merging threads.
> > > 
> > > On Fri, Nov 18, 2022 at 10:55:04AM +0100, Phil Sutter wrote:
> > > [...]
> > > > > I think this more or less a summary of what we discussed in the NFWS.
> > > >
> > > > Pablo, I think you're mixing up two things here:
> > > >
> > > > This "support dump and load of compat expression" feature is to sanitize
> > > > the current situation with up to date iptables and nftables.
> > > 
> > > OK, then the problem we discuss is mixing iptables-nft and nftables.
> > > 
> > > On Fri, Nov 18, 2022 at 10:47:48AM +0100, Phil Sutter wrote:
> > > [...]
> > > > > At this time I'd rather like a time machine to prevent nft_compat.c from
> > > > > getting merged :-(
> > > >
> > > > If you do, please convince Pablo to not push iptables commit 384958620a.
> > > > I think it opened the can of worms we're trying to confine here.
> > > 
> > > It could be worst, if iptables-nft would not be in place, then old
> > > iptables-legacy and new nftables rules would have no visibility each
> > > other.
> > > 
> > > With iptables-nft we have a way to move forward:
> > > 
> > > - Replace nft_compat by native expressions from iptables-nft.
> > > - Extend iptables-nft to understand more complex expressions, worst
> > >   case dump a native representation.
> > > 
> > > Why don't we just move ahead this path instead of spinning around the
> > > compat layer? This only requires userspace updates on iptables-nft.
> > 
> > Sure! I'm just picking low hanging fruits first. With even translation
> > support being still incomplete, I fear it will take a while until the
> > tools are fluent enough for this to not matter anymore. And then there's
> > still nftables without libxtables support.
> 
> Then perhaps its better to do following path:
> 1. Try ->xlate(), if that fails, then print a 'breaking' format?
> 
> As far as I understand the problem is the "# comment" - type syntax that
> makes nft just skip the incomplete rule, so perhaps just use invalid
> format?
> 
> Example:
> 
> counter packets 0 bytes 0 # name foo interval 250.0ms ewmalog 500.0ms
> Instead make this something like
> counter packets 0 bytes 0 nft_compat [ RATEEST name foo interval 250.0ms ewmalog 500.0ms ] # unsupported iptables-nft rule
> 
> ?
> 
> I'd like to avoid exposure in the frontend with compatible-restore-approach if possible.

Yes, that's fine with me. Now what about translated expressions? Can we
apply my warning patch until at least the majority of them is understood
by iptables?

Cheers, Phil

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-18 12:12           ` Phil Sutter
@ 2022-11-18 12:18             ` Pablo Neira Ayuso
  2022-11-18 12:24               ` Phil Sutter
  0 siblings, 1 reply; 18+ messages in thread
From: Pablo Neira Ayuso @ 2022-11-18 12:18 UTC (permalink / raw)
  To: Phil Sutter, Florian Westphal, netfilter-devel

On Fri, Nov 18, 2022 at 01:12:26PM +0100, Phil Sutter wrote:
> On Fri, Nov 18, 2022 at 12:46:43PM +0100, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > On Fri, Nov 18, 2022 at 11:11:42AM +0100, Pablo Neira Ayuso wrote:
> > > > Merging threads.
> > > > 
> > > > On Fri, Nov 18, 2022 at 10:55:04AM +0100, Phil Sutter wrote:
> > > > [...]
> > > > > > I think this more or less a summary of what we discussed in the NFWS.
> > > > >
> > > > > Pablo, I think you're mixing up two things here:
> > > > >
> > > > > This "support dump and load of compat expression" feature is to sanitize
> > > > > the current situation with up to date iptables and nftables.
> > > > 
> > > > OK, then the problem we discuss is mixing iptables-nft and nftables.
> > > > 
> > > > On Fri, Nov 18, 2022 at 10:47:48AM +0100, Phil Sutter wrote:
> > > > [...]
> > > > > > At this time I'd rather like a time machine to prevent nft_compat.c from
> > > > > > getting merged :-(
> > > > >
> > > > > If you do, please convince Pablo to not push iptables commit 384958620a.
> > > > > I think it opened the can of worms we're trying to confine here.
> > > > 
> > > > It could be worst, if iptables-nft would not be in place, then old
> > > > iptables-legacy and new nftables rules would have no visibility each
> > > > other.
> > > > 
> > > > With iptables-nft we have a way to move forward:
> > > > 
> > > > - Replace nft_compat by native expressions from iptables-nft.
> > > > - Extend iptables-nft to understand more complex expressions, worst
> > > >   case dump a native representation.
> > > > 
> > > > Why don't we just move ahead this path instead of spinning around the
> > > > compat layer? This only requires userspace updates on iptables-nft.
> > > 
> > > Sure! I'm just picking low hanging fruits first. With even translation
> > > support being still incomplete, I fear it will take a while until the
> > > tools are fluent enough for this to not matter anymore. And then there's
> > > still nftables without libxtables support.
> > 
> > Then perhaps its better to do following path:
> > 1. Try ->xlate(), if that fails, then print a 'breaking' format?
> > 
> > As far as I understand the problem is the "# comment" - type syntax that
> > makes nft just skip the incomplete rule, so perhaps just use invalid
> > format?
> > 
> > Example:
> > 
> > counter packets 0 bytes 0 # name foo interval 250.0ms ewmalog 500.0ms
> > Instead make this something like
> > counter packets 0 bytes 0 nft_compat [ RATEEST name foo interval 250.0ms ewmalog 500.0ms ] # unsupported iptables-nft rule
> > 
> > ?
> > 
> > I'd like to avoid exposure in the frontend with compatible-restore-approach if possible.
> 
> Yes, that's fine with me. Now what about translated expressions? Can we
> apply my warning patch until at least the majority of them is understood
> by iptables?

Which one are you refering to?

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-18 12:18             ` Pablo Neira Ayuso
@ 2022-11-18 12:24               ` Phil Sutter
  2022-11-18 13:34                 ` Florian Westphal
  0 siblings, 1 reply; 18+ messages in thread
From: Phil Sutter @ 2022-11-18 12:24 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

On Fri, Nov 18, 2022 at 01:18:44PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Nov 18, 2022 at 01:12:26PM +0100, Phil Sutter wrote:
> > On Fri, Nov 18, 2022 at 12:46:43PM +0100, Florian Westphal wrote:
> > > Phil Sutter <phil@nwl.cc> wrote:
> > > > On Fri, Nov 18, 2022 at 11:11:42AM +0100, Pablo Neira Ayuso wrote:
> > > > > Merging threads.
> > > > > 
> > > > > On Fri, Nov 18, 2022 at 10:55:04AM +0100, Phil Sutter wrote:
> > > > > [...]
> > > > > > > I think this more or less a summary of what we discussed in the NFWS.
> > > > > >
> > > > > > Pablo, I think you're mixing up two things here:
> > > > > >
> > > > > > This "support dump and load of compat expression" feature is to sanitize
> > > > > > the current situation with up to date iptables and nftables.
> > > > > 
> > > > > OK, then the problem we discuss is mixing iptables-nft and nftables.
> > > > > 
> > > > > On Fri, Nov 18, 2022 at 10:47:48AM +0100, Phil Sutter wrote:
> > > > > [...]
> > > > > > > At this time I'd rather like a time machine to prevent nft_compat.c from
> > > > > > > getting merged :-(
> > > > > >
> > > > > > If you do, please convince Pablo to not push iptables commit 384958620a.
> > > > > > I think it opened the can of worms we're trying to confine here.
> > > > > 
> > > > > It could be worst, if iptables-nft would not be in place, then old
> > > > > iptables-legacy and new nftables rules would have no visibility each
> > > > > other.
> > > > > 
> > > > > With iptables-nft we have a way to move forward:
> > > > > 
> > > > > - Replace nft_compat by native expressions from iptables-nft.
> > > > > - Extend iptables-nft to understand more complex expressions, worst
> > > > >   case dump a native representation.
> > > > > 
> > > > > Why don't we just move ahead this path instead of spinning around the
> > > > > compat layer? This only requires userspace updates on iptables-nft.
> > > > 
> > > > Sure! I'm just picking low hanging fruits first. With even translation
> > > > support being still incomplete, I fear it will take a while until the
> > > > tools are fluent enough for this to not matter anymore. And then there's
> > > > still nftables without libxtables support.
> > > 
> > > Then perhaps its better to do following path:
> > > 1. Try ->xlate(), if that fails, then print a 'breaking' format?
> > > 
> > > As far as I understand the problem is the "# comment" - type syntax that
> > > makes nft just skip the incomplete rule, so perhaps just use invalid
> > > format?
> > > 
> > > Example:
> > > 
> > > counter packets 0 bytes 0 # name foo interval 250.0ms ewmalog 500.0ms
> > > Instead make this something like
> > > counter packets 0 bytes 0 nft_compat [ RATEEST name foo interval 250.0ms ewmalog 500.0ms ] # unsupported iptables-nft rule
> > > 
> > > ?
> > > 
> > > I'd like to avoid exposure in the frontend with compatible-restore-approach if possible.
> > 
> > Yes, that's fine with me. Now what about translated expressions? Can we
> > apply my warning patch until at least the majority of them is understood
> > by iptables?
> 
> Which one are you refering to?

This one:

Subject: [nft PATCH] Warn for tables with compat expressions in rules
Date: Wed, 12 Oct 2022 17:31:07 +0200
Message-Id: <20221012153107.24574-1-phil@nwl.cc>

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-18 12:24               ` Phil Sutter
@ 2022-11-18 13:34                 ` Florian Westphal
  2022-11-18 14:10                   ` Pablo Neira Ayuso
  0 siblings, 1 reply; 18+ messages in thread
From: Florian Westphal @ 2022-11-18 13:34 UTC (permalink / raw)
  To: Phil Sutter, Pablo Neira Ayuso, Florian Westphal, netfilter-devel

Phil Sutter <phil@nwl.cc> wrote:
> This one:
> 
> Subject: [nft PATCH] Warn for tables with compat expressions in rules
> Date: Wed, 12 Oct 2022 17:31:07 +0200
> Message-Id: <20221012153107.24574-1-phil@nwl.cc>

This is:
https://patchwork.ozlabs.org/project/netfilter-devel/patch/20221012153107.24574-1-phil@nwl.cc/

LGTM, no objections from me.

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-18 13:34                 ` Florian Westphal
@ 2022-11-18 14:10                   ` Pablo Neira Ayuso
  2022-11-18 14:52                     ` Phil Sutter
  0 siblings, 1 reply; 18+ messages in thread
From: Pablo Neira Ayuso @ 2022-11-18 14:10 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Phil Sutter, netfilter-devel

On Fri, Nov 18, 2022 at 02:34:31PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > This one:
> > 
> > Subject: [nft PATCH] Warn for tables with compat expressions in rules
> > Date: Wed, 12 Oct 2022 17:31:07 +0200
> > Message-Id: <20221012153107.24574-1-phil@nwl.cc>
> 
> This is:
> https://patchwork.ozlabs.org/project/netfilter-devel/patch/20221012153107.24574-1-phil@nwl.cc/
> 
> LGTM, no objections from me.

LGTM.

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

* Re: [nft PATCH v2 0/4] xt: Implement dump and restore support
  2022-11-18 14:10                   ` Pablo Neira Ayuso
@ 2022-11-18 14:52                     ` Phil Sutter
  0 siblings, 0 replies; 18+ messages in thread
From: Phil Sutter @ 2022-11-18 14:52 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

On Fri, Nov 18, 2022 at 03:10:54PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Nov 18, 2022 at 02:34:31PM +0100, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > This one:
> > > 
> > > Subject: [nft PATCH] Warn for tables with compat expressions in rules
> > > Date: Wed, 12 Oct 2022 17:31:07 +0200
> > > Message-Id: <20221012153107.24574-1-phil@nwl.cc>
> > 
> > This is:
> > https://patchwork.ozlabs.org/project/netfilter-devel/patch/20221012153107.24574-1-phil@nwl.cc/
> > 
> > LGTM, no objections from me.
> 
> LGTM.

Applied, thanks!

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

end of thread, other threads:[~2022-11-18 14:52 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-17 17:45 [nft PATCH v2 0/4] xt: Implement dump and restore support Phil Sutter
2022-11-17 17:45 ` [nft PATCH v2 1/4] xt: Delay libxtables access until translation Phil Sutter
2022-11-17 17:45 ` [nft PATCH v2 2/4] xt: Implement dump and restore support Phil Sutter
2022-11-17 17:45 ` [nft PATCH v2 3/4] xt: Put match/target translation into own functions Phil Sutter
2022-11-17 17:45 ` [nft PATCH v2 4/4] xt: Detect xlate callback failure Phil Sutter
2022-11-17 21:13 ` [nft PATCH v2 0/4] xt: Implement dump and restore support Florian Westphal
2022-11-18  9:40   ` Pablo Neira Ayuso
2022-11-18  9:55     ` Phil Sutter
2022-11-18  9:47   ` Phil Sutter
2022-11-18 10:11     ` Pablo Neira Ayuso
2022-11-18 10:42       ` Phil Sutter
2022-11-18 11:46         ` Florian Westphal
2022-11-18 12:12           ` Phil Sutter
2022-11-18 12:18             ` Pablo Neira Ayuso
2022-11-18 12:24               ` Phil Sutter
2022-11-18 13:34                 ` Florian Westphal
2022-11-18 14:10                   ` Pablo Neira Ayuso
2022-11-18 14:52                     ` Phil Sutter

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.