netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [iptables PATCH 00/13] Extensions: Review xlate callbacks
@ 2022-11-23 16:43 Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 01/13] extensions: libebt_mark: Fix mark target xlate Phil Sutter
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

This is a review of all extensions' xlate callbacks focussing on
correctness of operation and return code.

Patches 1 and 2 address ebtables mark target.
Patches 3, 4, 6 and 9 add 'return 0' calls in more or less unlikely
cases.
Patch 5 fixes multiple problems in CONNMARK target.
Patches 7, 8 and 10 extend some translations.
The last three patches add comments to help future reviewers asserting
code correctness.

Phil Sutter (13):
  extensions: libebt_mark: Fix mark target xlate
  extensions: libebt_mark: Fix xlate test case
  extensions: libebt_redirect: Fix xlate return code
  extensions: libipt_ttl: Sanitize xlate callback
  extensions: CONNMARK: Fix xlate callback
  extensions: MARK: Sanitize MARK_xlate()
  extensions: TCPMSS: Use xlate callback for IPv6, too
  extensions: TOS: Fix v1 xlate callback
  extensions: ecn: Sanitize xlate callback
  extensions: tcp: Translate TCP option match
  extensions: libebt_log: Add comment to clarify xlate callback
  extensions: frag: Add comment to clarify xlate callback
  extensions: ipcomp: Add comment to clarify xlate callback

 extensions/libebt_log.c          |  2 ++
 extensions/libebt_mark.c         |  2 +-
 extensions/libebt_mark.txlate    | 11 +++++++++++
 extensions/libebt_mark.xlate     | 11 -----------
 extensions/libebt_redirect.c     |  2 +-
 extensions/libip6t_frag.c        |  2 ++
 extensions/libipt_ttl.c          |  4 ++--
 extensions/libxt_CONNMARK.c      | 15 ++++++++++-----
 extensions/libxt_CONNMARK.txlate |  3 +++
 extensions/libxt_MARK.c          |  2 ++
 extensions/libxt_TCPMSS.c        |  1 +
 extensions/libxt_TOS.c           | 33 ++++++++++++++++++++++----------
 extensions/libxt_TOS.txlate      |  9 ++++++---
 extensions/libxt_ecn.c           |  2 ++
 extensions/libxt_ipcomp.c        |  2 ++
 extensions/libxt_ipcomp.c.man    |  3 ---
 extensions/libxt_tcp.c           |  9 ++++++---
 extensions/libxt_tcp.txlate      |  6 ++++++
 18 files changed, 80 insertions(+), 39 deletions(-)
 create mode 100644 extensions/libebt_mark.txlate
 delete mode 100644 extensions/libebt_mark.xlate

-- 
2.38.0


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

* [iptables PATCH 01/13] extensions: libebt_mark: Fix mark target xlate
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 02/13] extensions: libebt_mark: Fix xlate test case Phil Sutter
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

Target value is constructed setting all non-target bits to one instead
of zero.

Fixes: 03ecffe6c2cc0 ("ebtables-compat: add initial translations")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libebt_mark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extensions/libebt_mark.c b/extensions/libebt_mark.c
index 423c5c9133d0d..40e49618e0215 100644
--- a/extensions/libebt_mark.c
+++ b/extensions/libebt_mark.c
@@ -201,7 +201,7 @@ static int brmark_xlate(struct xt_xlate *xl,
 		return 0;
 	}
 
-	tmp = info->target & EBT_VERDICT_BITS;
+	tmp = info->target | ~EBT_VERDICT_BITS;
 	xt_xlate_add(xl, "0x%lx %s ", info->mark, brmark_verdict(tmp));
 	return 1;
 }
-- 
2.38.0


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

* [iptables PATCH 02/13] extensions: libebt_mark: Fix xlate test case
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 01/13] extensions: libebt_mark: Fix mark target xlate Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 03/13] extensions: libebt_redirect: Fix xlate return code Phil Sutter
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

The false suffix effectively disabled this test file, but it also has
problems: Apart from brmark_xlate() printing 'meta mark' instead of just
'mark', target is printed in the wrong position (like with any other
target-possessing extension.

Fixes: e67c08880961f ("ebtables-translate: add initial test cases")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libebt_mark.txlate | 11 +++++++++++
 extensions/libebt_mark.xlate  | 11 -----------
 2 files changed, 11 insertions(+), 11 deletions(-)
 create mode 100644 extensions/libebt_mark.txlate
 delete mode 100644 extensions/libebt_mark.xlate

diff --git a/extensions/libebt_mark.txlate b/extensions/libebt_mark.txlate
new file mode 100644
index 0000000000000..7529302d9a444
--- /dev/null
+++ b/extensions/libebt_mark.txlate
@@ -0,0 +1,11 @@
+ebtables-translate -A INPUT --mark-set 42
+nft add rule bridge filter INPUT meta mark set 0x2a accept counter
+
+ebtables-translate -A INPUT --mark-or 42 --mark-target RETURN
+nft add rule bridge filter INPUT meta mark set meta mark or 0x2a return counter
+
+ebtables-translate -A INPUT --mark-and 42 --mark-target ACCEPT
+nft add rule bridge filter INPUT meta mark set meta mark and 0x2a accept counter
+
+ebtables-translate -A INPUT --mark-xor 42 --mark-target DROP
+nft add rule bridge filter INPUT meta mark set meta mark xor 0x2a drop counter
diff --git a/extensions/libebt_mark.xlate b/extensions/libebt_mark.xlate
deleted file mode 100644
index e0982a1e8ebd7..0000000000000
--- a/extensions/libebt_mark.xlate
+++ /dev/null
@@ -1,11 +0,0 @@
-ebtables-translate -A INPUT --mark-set 42
-nft add rule bridge filter INPUT mark set 0x2a counter
-
-ebtables-translate -A INPUT --mark-or 42 --mark-target RETURN
-nft add rule bridge filter INPUT mark set mark or 0x2a counter return
-
-ebtables-translate -A INPUT --mark-and 42 --mark-target ACCEPT
-nft add rule bridge filter INPUT mark set mark and 0x2a counter accept
-
-ebtables-translate -A INPUT --mark-xor 42 --mark-target DROP
-nft add rule bridge filter INPUT mark set mark xor 0x2a counter drop
-- 
2.38.0


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

* [iptables PATCH 03/13] extensions: libebt_redirect: Fix xlate return code
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 01/13] extensions: libebt_mark: Fix mark target xlate Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 02/13] extensions: libebt_mark: Fix xlate test case Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 04/13] extensions: libipt_ttl: Sanitize xlate callback Phil Sutter
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

The callback is supposed to return 1 on success, not 0.

Fixes: 24ce7465056ae ("ebtables-compat: add redirect match extension")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libebt_redirect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extensions/libebt_redirect.c b/extensions/libebt_redirect.c
index 6e653997ee99e..4d4c7a02cea89 100644
--- a/extensions/libebt_redirect.c
+++ b/extensions/libebt_redirect.c
@@ -86,7 +86,7 @@ static int brredir_xlate(struct xt_xlate *xl,
 	xt_xlate_add(xl, "meta set pkttype host");
 	if (red->target != EBT_ACCEPT)
 		xt_xlate_add(xl, " %s ", brredir_verdict(red->target));
-	return 0;
+	return 1;
 }
 
 static struct xtables_target brredirect_target = {
-- 
2.38.0


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

* [iptables PATCH 04/13] extensions: libipt_ttl: Sanitize xlate callback
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (2 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 03/13] extensions: libebt_redirect: Fix xlate return code Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 05/13] extensions: CONNMARK: Fix " Phil Sutter
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

Catch unexpected values in info->mode, also fix indenting.

Fixes: 1b320a1a1dc1f ("extensions: libipt_ttl: Add translation to nft")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libipt_ttl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/extensions/libipt_ttl.c b/extensions/libipt_ttl.c
index 6bdd219618091..86ba554ef92a8 100644
--- a/extensions/libipt_ttl.c
+++ b/extensions/libipt_ttl.c
@@ -106,7 +106,7 @@ static int ttl_xlate(struct xt_xlate *xl,
 	const struct ipt_ttl_info *info =
 		(struct ipt_ttl_info *) params->match->data;
 
-		switch (info->mode) {
+	switch (info->mode) {
 		case IPT_TTL_EQ:
 			xt_xlate_add(xl, "ip ttl");
 			break;
@@ -121,7 +121,7 @@ static int ttl_xlate(struct xt_xlate *xl,
 			break;
 		default:
 			/* Should not happen. */
-			break;
+			return 0;
 	}
 
 	xt_xlate_add(xl, " %u", info->ttl);
-- 
2.38.0


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

* [iptables PATCH 05/13] extensions: CONNMARK: Fix xlate callback
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (3 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 04/13] extensions: libipt_ttl: Sanitize xlate callback Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 06/13] extensions: MARK: Sanitize MARK_xlate() Phil Sutter
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

Bail out if nfmask != ctmask with XT_CONNMARK_SAVE and
XT_CONNMARK_RESTORE. Looks like this needs a similar implementation to
the one for XT_CONNMARK_SET.

Fix shift mark translation: xt_connmark_shift_ops does not contain
useful strings for nftables. Also add needed braces around the term
being shifted.

Fixes: db7b4e0de960c ("extensions: libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_CONNMARK.c      | 15 ++++++++++-----
 extensions/libxt_CONNMARK.txlate |  3 +++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c
index 21e1091386294..a6568c99b6c4d 100644
--- a/extensions/libxt_CONNMARK.c
+++ b/extensions/libxt_CONNMARK.c
@@ -595,11 +595,11 @@ static int connmark_tg_xlate_v2(struct xt_xlate *xl,
 {
 	const struct xt_connmark_tginfo2 *info =
 		(const void *)params->target->data;
-	const char *shift_op = xt_connmark_shift_ops[info->shift_dir];
+	const char *braces = info->shift_bits ? "( " : "";
 
 	switch (info->mode) {
 	case XT_CONNMARK_SET:
-		xt_xlate_add(xl, "ct mark set ");
+		xt_xlate_add(xl, "ct mark set %s", braces);
 		if (info->ctmask == 0xFFFFFFFFU)
 			xt_xlate_add(xl, "0x%x ", info->ctmark);
 		else if (info->ctmark == 0)
@@ -615,26 +615,31 @@ static int connmark_tg_xlate_v2(struct xt_xlate *xl,
 				     info->ctmark, ~info->ctmask);
 		break;
 	case XT_CONNMARK_SAVE:
-		xt_xlate_add(xl, "ct mark set mark");
+		xt_xlate_add(xl, "ct mark set %smark", braces);
 		if (!(info->nfmask == UINT32_MAX &&
 		    info->ctmask == UINT32_MAX)) {
 			if (info->nfmask == info->ctmask)
 				xt_xlate_add(xl, " and 0x%x", info->nfmask);
+			else
+				return 0;
 		}
 		break;
 	case XT_CONNMARK_RESTORE:
-		xt_xlate_add(xl, "meta mark set ct mark");
+		xt_xlate_add(xl, "meta mark set %sct mark", braces);
 		if (!(info->nfmask == UINT32_MAX &&
 		    info->ctmask == UINT32_MAX)) {
 			if (info->nfmask == info->ctmask)
 				xt_xlate_add(xl, " and 0x%x", info->nfmask);
+			else
+				return 0;
 		}
 		break;
 	}
 
 	if (info->mode <= XT_CONNMARK_RESTORE &&
 	    info->shift_bits != 0) {
-		xt_xlate_add(xl, " %s %u", shift_op, info->shift_bits);
+		xt_xlate_add(xl, " ) %s %u",
+			     info->shift_dir ? ">>" : "<<", info->shift_bits);
 	}
 
 	return 1;
diff --git a/extensions/libxt_CONNMARK.txlate b/extensions/libxt_CONNMARK.txlate
index ce40ae5ea65e0..99627c2b05d45 100644
--- a/extensions/libxt_CONNMARK.txlate
+++ b/extensions/libxt_CONNMARK.txlate
@@ -18,3 +18,6 @@ nft add rule ip mangle PREROUTING counter ct mark set mark
 
 iptables-translate -t mangle -A PREROUTING -j CONNMARK --restore-mark
 nft add rule ip mangle PREROUTING counter meta mark set ct mark
+
+iptables-translate -t mangle -A PREROUTING  -j CONNMARK --set-mark 0x23/0x42 --right-shift-mark 5
+nft add rule ip mangle PREROUTING counter ct mark set ( ct mark xor 0x23 and 0xffffff9c ) >> 5
-- 
2.38.0


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

* [iptables PATCH 06/13] extensions: MARK: Sanitize MARK_xlate()
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (4 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 05/13] extensions: CONNMARK: Fix " Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 07/13] extensions: TCPMSS: Use xlate callback for IPv6, too Phil Sutter
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

Since markinfo->mode might contain unexpected values, add a default case
returning zero.

Fixes: afefc7a134ca0 ("extensions: libxt_MARK: Add translation for revision 1 to nft")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_MARK.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/extensions/libxt_MARK.c b/extensions/libxt_MARK.c
index 1536563d0f4c7..100f6a38996ac 100644
--- a/extensions/libxt_MARK.c
+++ b/extensions/libxt_MARK.c
@@ -366,6 +366,8 @@ static int MARK_xlate(struct xt_xlate *xl,
 	case XT_MARK_OR:
 		xt_xlate_add(xl, "mark or 0x%x ", (uint32_t)markinfo->mark);
 		break;
+	default:
+		return 0;
 	}
 
 	return 1;
-- 
2.38.0


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

* [iptables PATCH 07/13] extensions: TCPMSS: Use xlate callback for IPv6, too
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (5 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 06/13] extensions: MARK: Sanitize MARK_xlate() Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 08/13] extensions: TOS: Fix v1 xlate callback Phil Sutter
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

Data structures are identical and the translation is layer3-agnostic.

Fixes: bebce197adb42 ("iptables: iptables-compat translation for TCPMSS")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_TCPMSS.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/extensions/libxt_TCPMSS.c b/extensions/libxt_TCPMSS.c
index 0d9b200ebc72f..251a5532a838b 100644
--- a/extensions/libxt_TCPMSS.c
+++ b/extensions/libxt_TCPMSS.c
@@ -131,6 +131,7 @@ static struct xtables_target tcpmss_tg_reg[] = {
 		.x6_parse      = TCPMSS_parse,
 		.x6_fcheck     = TCPMSS_check,
 		.x6_options    = TCPMSS6_opts,
+		.xlate         = TCPMSS_xlate,
 	},
 };
 
-- 
2.38.0


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

* [iptables PATCH 08/13] extensions: TOS: Fix v1 xlate callback
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (6 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 07/13] extensions: TCPMSS: Use xlate callback for IPv6, too Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 09/13] extensions: ecn: Sanitize " Phil Sutter
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

Translation entirely ignored tos_mask field.

Fixes: b669e18489709 ("extensions: libxt_TOS: Add translation to nft")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_TOS.c      | 33 +++++++++++++++++++++++----------
 extensions/libxt_TOS.txlate |  9 ++++++---
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/extensions/libxt_TOS.c b/extensions/libxt_TOS.c
index b66fa329f4150..4fc849bd2468b 100644
--- a/extensions/libxt_TOS.c
+++ b/extensions/libxt_TOS.c
@@ -183,28 +183,41 @@ static void tos_tg_save(const void *ip, const struct xt_entry_target *target)
 	printf(" --set-tos 0x%02x/0x%02x", info->tos_value, info->tos_mask);
 }
 
+static int __tos_xlate(struct xt_xlate *xl, const char *ip,
+		       uint8_t tos, uint8_t tosmask)
+{
+	xt_xlate_add(xl, "%s dscp set ", ip);
+	if ((tosmask & 0x3f) == 0x3f)
+		xt_xlate_add(xl, "0x%02x", tos >> 2);
+	else if (!tos)
+		xt_xlate_add(xl, "%s dscp and 0x%02x",
+			     ip, (uint8_t)~tosmask >> 2);
+	else if (tos == tosmask)
+		xt_xlate_add(xl, "%s dscp or 0x%02x", ip, tos >> 2);
+	else if (!tosmask)
+		xt_xlate_add(xl, "%s dscp xor 0x%02x", ip, tos >> 2);
+	else
+		xt_xlate_add(xl, "%s dscp and 0x%02x xor 0x%02x",
+			     ip, (uint8_t)~tosmask >> 2, tos >> 2);
+	return 1;
+}
+
 static int tos_xlate(struct xt_xlate *xl,
 		     const struct xt_xlate_tg_params *params)
 {
 	const struct ipt_tos_target_info *info =
 			(struct ipt_tos_target_info *) params->target->data;
-	uint8_t dscp = info->tos >> 2;
-
-	xt_xlate_add(xl, "ip dscp set 0x%02x", dscp);
 
-	return 1;
+	return __tos_xlate(xl, "ip", info->tos, UINT8_MAX);
 }
 
 static int tos_xlate6(struct xt_xlate *xl,
 		     const struct xt_xlate_tg_params *params)
 {
-	const struct ipt_tos_target_info *info =
-			(struct ipt_tos_target_info *) params->target->data;
-	uint8_t dscp = info->tos >> 2;
+	const struct xt_tos_target_info *info =
+			(struct xt_tos_target_info *)params->target->data;
 
-	xt_xlate_add(xl, "ip6 dscp set 0x%02x", dscp);
-
-	return 1;
+	return __tos_xlate(xl, "ip6", info->tos_value, info->tos_mask);
 }
 
 static struct xtables_target tos_tg_reg[] = {
diff --git a/extensions/libxt_TOS.txlate b/extensions/libxt_TOS.txlate
index 0952310edc4ac..9c12674299359 100644
--- a/extensions/libxt_TOS.txlate
+++ b/extensions/libxt_TOS.txlate
@@ -14,10 +14,13 @@ ip6tables-translate -A INPUT -j TOS --set-tos Normal-Service
 nft add rule ip6 filter INPUT counter ip6 dscp set 0x00
 
 ip6tables-translate -A INPUT -j TOS --and-tos 0x12
-nft add rule ip6 filter INPUT counter ip6 dscp set 0x00
+nft add rule ip6 filter INPUT counter ip6 dscp set ip6 dscp and 0x04
 
 ip6tables-translate -A INPUT -j TOS --or-tos 0x12
-nft add rule ip6 filter INPUT counter ip6 dscp set 0x04
+nft add rule ip6 filter INPUT counter ip6 dscp set ip6 dscp or 0x04
 
 ip6tables-translate -A INPUT -j TOS --xor-tos 0x12
-nft add rule ip6 filter INPUT counter ip6 dscp set 0x04
+nft add rule ip6 filter INPUT counter ip6 dscp set ip6 dscp xor 0x04
+
+ip6tables-translate -A INPUT -j TOS --set-tos 0x12/0x34
+nft add rule ip6 filter INPUT counter ip6 dscp set ip6 dscp and 0x32 xor 0x04
-- 
2.38.0


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

* [iptables PATCH 09/13] extensions: ecn: Sanitize xlate callback
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (7 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 08/13] extensions: TOS: Fix v1 xlate callback Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 10/13] extensions: tcp: Translate TCP option match Phil Sutter
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

Catch unexpected values in einfo->ip_ect.

Fixes: ca42442093d3d ("iptables: extensions: libxt_ecn: Add translation to nft")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_ecn.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/extensions/libxt_ecn.c b/extensions/libxt_ecn.c
index ad3c7a0307a0d..83a4acfab7da7 100644
--- a/extensions/libxt_ecn.c
+++ b/extensions/libxt_ecn.c
@@ -156,6 +156,8 @@ static int ecn_xlate(struct xt_xlate *xl,
 		case 3:
 			xt_xlate_add(xl, "ce");
 			break;
+		default:
+			return 0;
 		}
 	}
 	return 1;
-- 
2.38.0


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

* [iptables PATCH 10/13] extensions: tcp: Translate TCP option match
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (8 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 09/13] extensions: ecn: Sanitize " Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 11/13] extensions: libebt_log: Add comment to clarify xlate callback Phil Sutter
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

A simple task since 'tcp option' expression exists.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_tcp.c      | 9 ++++++---
 extensions/libxt_tcp.txlate | 6 ++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/extensions/libxt_tcp.c b/extensions/libxt_tcp.c
index 0b115cddf15d9..043382d47b8ba 100644
--- a/extensions/libxt_tcp.c
+++ b/extensions/libxt_tcp.c
@@ -430,9 +430,12 @@ static int tcp_xlate(struct xt_xlate *xl,
 		space = " ";
 	}
 
-	/* XXX not yet implemented */
-	if (tcpinfo->option || (tcpinfo->invflags & XT_TCP_INV_OPTION))
-		return 0;
+	if (tcpinfo->option) {
+		xt_xlate_add(xl, "%stcp option %u %s", space, tcpinfo->option,
+			     tcpinfo->invflags & XT_TCP_INV_OPTION ?
+			     "missing" : "exists");
+		space = " ";
+	}
 
 	if (tcpinfo->flg_mask || (tcpinfo->invflags & XT_TCP_INV_FLAGS)) {
 		xt_xlate_add(xl, "%stcp flags %s", space,
diff --git a/extensions/libxt_tcp.txlate b/extensions/libxt_tcp.txlate
index 921d4af024d32..a1f0e909bb46c 100644
--- a/extensions/libxt_tcp.txlate
+++ b/extensions/libxt_tcp.txlate
@@ -24,3 +24,9 @@ nft add rule ip filter INPUT ip frag-off & 0x1fff != 0 ip protocol tcp counter
 
 iptables-translate -A INPUT ! -f -p tcp --dport 22
 nft add rule ip filter INPUT ip frag-off & 0x1fff 0 tcp dport 22 counter
+
+iptables-translate -A INPUT -p tcp --tcp-option 23
+nft add rule ip filter INPUT tcp option 23 exists counter
+
+iptables-translate -A INPUT -p tcp ! --tcp-option 23
+nft add rule ip filter INPUT tcp option 23 missing counter
-- 
2.38.0


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

* [iptables PATCH 11/13] extensions: libebt_log: Add comment to clarify xlate callback
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (9 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 10/13] extensions: tcp: Translate TCP option match Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 12/13] extensions: frag: " Phil Sutter
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

Several log flags are ignored by the function. Add a comment explaining
why this is correct.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libebt_log.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/extensions/libebt_log.c b/extensions/libebt_log.c
index 47708d79310e0..13c7fafecb11e 100644
--- a/extensions/libebt_log.c
+++ b/extensions/libebt_log.c
@@ -191,6 +191,8 @@ static int brlog_xlate(struct xt_xlate *xl,
 	if (loginfo->loglevel != LOG_DEFAULT_LEVEL)
 		xt_xlate_add(xl, " level %s", eight_priority[loginfo->loglevel].c_name);
 
+	/* ebt_log always decodes MAC header, nft_log always decodes upper header -
+	 * so set flags ether and ignore EBT_LOG_IP, EBT_LOG_ARP and EBT_LOG_IP6 */
 	xt_xlate_add(xl, " flags ether ");
 
 	return 1;
-- 
2.38.0


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

* [iptables PATCH 12/13] extensions: frag: Add comment to clarify xlate callback
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (10 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 11/13] extensions: libebt_log: Add comment to clarify xlate callback Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 16:43 ` [iptables PATCH 13/13] extensions: ipcomp: " Phil Sutter
  2022-11-23 21:26 ` [iptables PATCH 00/13] Extensions: Review xlate callbacks Florian Westphal
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

Matching on fragmentation header length is ineffective in kernel, xlate
callback correctly ignores it. Add a comment as a hint for reviewers.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libip6t_frag.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/extensions/libip6t_frag.c b/extensions/libip6t_frag.c
index 3842496e56a55..72a43153c53dc 100644
--- a/extensions/libip6t_frag.c
+++ b/extensions/libip6t_frag.c
@@ -193,6 +193,8 @@ static int frag_xlate(struct xt_xlate *xl,
 		space = " ";
 	}
 
+	/* ignore ineffective IP6T_FRAG_LEN bit */
+
 	if (fraginfo->flags & IP6T_FRAG_RES) {
 		xt_xlate_add(xl, "%sfrag reserved 1", space);
 		space = " ";
-- 
2.38.0


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

* [iptables PATCH 13/13] extensions: ipcomp: Add comment to clarify xlate callback
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (11 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 12/13] extensions: frag: " Phil Sutter
@ 2022-11-23 16:43 ` Phil Sutter
  2022-11-23 21:26 ` [iptables PATCH 00/13] Extensions: Review xlate callbacks Florian Westphal
  13 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-23 16:43 UTC (permalink / raw)
  To: netfilter-devel

Kernel ignores 'hdrres' field, this matching on reserved field value was
never effective.

While being at it, drop its description from man page. Continue to parse
and print it for compatibility reasons, but avoid attracting new users.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_ipcomp.c     | 2 ++
 extensions/libxt_ipcomp.c.man | 3 ---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/extensions/libxt_ipcomp.c b/extensions/libxt_ipcomp.c
index b5c43128466fb..4171c4a1c4eb7 100644
--- a/extensions/libxt_ipcomp.c
+++ b/extensions/libxt_ipcomp.c
@@ -101,6 +101,8 @@ static int comp_xlate(struct xt_xlate *xl,
 	const struct xt_ipcomp *compinfo =
 		(struct xt_ipcomp *)params->match->data;
 
+	/* ignore compinfo->hdrres like kernel's xt_ipcomp.c does */
+
 	xt_xlate_add(xl, "comp cpi %s",
 		     compinfo->invflags & XT_IPCOMP_INV_SPI ? "!= " : "");
 	if (compinfo->spis[0] != compinfo->spis[1])
diff --git a/extensions/libxt_ipcomp.c.man b/extensions/libxt_ipcomp.c.man
index f3b17d2167697..824f5b3d9dbb4 100644
--- a/extensions/libxt_ipcomp.c.man
+++ b/extensions/libxt_ipcomp.c.man
@@ -2,6 +2,3 @@ This module matches the parameters in IPcomp header of IPsec packets.
 .TP
 [\fB!\fP] \fB\-\-ipcompspi\fP \fIspi\fP[\fB:\fP\fIspi\fP]
 Matches IPcomp header CPI value.
-.TP
-\fB\-\-compres\fP
-Matches if the reserved field is filled with zero.
-- 
2.38.0


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

* Re: [iptables PATCH 00/13] Extensions: Review xlate callbacks
  2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
                   ` (12 preceding siblings ...)
  2022-11-23 16:43 ` [iptables PATCH 13/13] extensions: ipcomp: " Phil Sutter
@ 2022-11-23 21:26 ` Florian Westphal
  2022-11-24  9:24   ` Phil Sutter
  13 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2022-11-23 21:26 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netfilter-devel

Phil Sutter <phil@nwl.cc> wrote:
> This is a review of all extensions' xlate callbacks focussing on
> correctness of operation and return code.

Thanks, please push this out.

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

* Re: [iptables PATCH 00/13] Extensions: Review xlate callbacks
  2022-11-23 21:26 ` [iptables PATCH 00/13] Extensions: Review xlate callbacks Florian Westphal
@ 2022-11-24  9:24   ` Phil Sutter
  0 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2022-11-24  9:24 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Wed, Nov 23, 2022 at 10:26:21PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > This is a review of all extensions' xlate callbacks focussing on
> > correctness of operation and return code.
> 
> Thanks, please push this out.

DONE!

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

end of thread, other threads:[~2022-11-24  9:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-23 16:43 [iptables PATCH 00/13] Extensions: Review xlate callbacks Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 01/13] extensions: libebt_mark: Fix mark target xlate Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 02/13] extensions: libebt_mark: Fix xlate test case Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 03/13] extensions: libebt_redirect: Fix xlate return code Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 04/13] extensions: libipt_ttl: Sanitize xlate callback Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 05/13] extensions: CONNMARK: Fix " Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 06/13] extensions: MARK: Sanitize MARK_xlate() Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 07/13] extensions: TCPMSS: Use xlate callback for IPv6, too Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 08/13] extensions: TOS: Fix v1 xlate callback Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 09/13] extensions: ecn: Sanitize " Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 10/13] extensions: tcp: Translate TCP option match Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 11/13] extensions: libebt_log: Add comment to clarify xlate callback Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 12/13] extensions: frag: " Phil Sutter
2022-11-23 16:43 ` [iptables PATCH 13/13] extensions: ipcomp: " Phil Sutter
2022-11-23 21:26 ` [iptables PATCH 00/13] Extensions: Review xlate callbacks Florian Westphal
2022-11-24  9:24   ` Phil Sutter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).