[iptables PATCH v2 1/2] extensions: libxt_conntrack: print xlate state as set

[iptables PATCH v2 1/2] extensions: libxt_conntrack: print xlate state as set

[Alexander Mikhalitsyn]

Currently, state_xlate_print function prints statemask
without { ... } around. But if ctstate condition is
negative, then we have to use { ... } after "!=" operator

Reproducer:
$ iptables -A INPUT -d 127.0.0.1/32 -p tcp -m conntrack ! --ctstate RELATED,ESTABLISHED -j DROP
$ nft list ruleset
...
meta l4proto tcp ip daddr 127.0.0.1 ct state != related,established counter packets 0 bytes 0 drop
...

it will fail if we try to load this rule:
$ nft -f nft_test
../nft_test:6:97-97: Error: syntax error, unexpected comma, expecting newline or semicolon

Signed-off-by: Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com>
---
 extensions/libxt_conntrack.c      | 18 +++++++++++++++---
 extensions/libxt_conntrack.txlate |  5 ++++-
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/extensions/libxt_conntrack.c b/extensions/libxt_conntrack.c
index 7734509..fe964aa 100644
--- a/extensions/libxt_conntrack.c
+++ b/extensions/libxt_conntrack.c
@@ -1148,9 +1148,16 @@ static void state_save(const void *ip, const struct xt_entry_match *match)
 	state_print_state(sinfo->statemask);
 }
 
-static void state_xlate_print(struct xt_xlate *xl, unsigned int statemask)
+static void state_xlate_print(struct xt_xlate *xl, unsigned int statemask, int afterinv)
 {
 	const char *sep = "";
+	int as_set;
+
+	/* print as set only after inversion and if more than one flag is set */
+	as_set = afterinv && (statemask & (statemask - 1));
+
+	if (as_set)
+		xt_xlate_add(xl, "{ ");
 
 	if (statemask & XT_CONNTRACK_STATE_INVALID) {
 		xt_xlate_add(xl, "%s%s", sep, "invalid");
@@ -1172,6 +1179,9 @@ static void state_xlate_print(struct xt_xlate *xl, unsigned int statemask)
 		xt_xlate_add(xl, "%s%s", sep, "untracked");
 		sep = ",";
 	}
+
+	if (as_set)
+		xt_xlate_add(xl, " }");
 }
 
 static int state_xlate(struct xt_xlate *xl,
@@ -1182,7 +1192,8 @@ static int state_xlate(struct xt_xlate *xl,
 
 	xt_xlate_add(xl, "ct state %s", sinfo->invert_flags & XT_CONNTRACK_STATE ?
 					"!= " : "");
-	state_xlate_print(xl, sinfo->state_mask);
+	state_xlate_print(xl, sinfo->state_mask,
+			  sinfo->invert_flags & XT_CONNTRACK_STATE);
 	xt_xlate_add(xl, " ");
 	return 1;
 }
@@ -1259,7 +1270,8 @@ static int _conntrack3_mt_xlate(struct xt_xlate *xl,
 			xt_xlate_add(xl, "%sct state %s", space,
 				     sinfo->invert_flags & XT_CONNTRACK_STATE ?
 				     "!= " : "");
-			state_xlate_print(xl, sinfo->state_mask);
+			state_xlate_print(xl, sinfo->state_mask,
+					  sinfo->invert_flags & XT_CONNTRACK_STATE);
 			space = " ";
 		}
 	}
diff --git a/extensions/libxt_conntrack.txlate b/extensions/libxt_conntrack.txlate
index d374f8a..75b3daa 100644
--- a/extensions/libxt_conntrack.txlate
+++ b/extensions/libxt_conntrack.txlate
@@ -2,7 +2,10 @@ iptables-translate -t filter -A INPUT -m conntrack --ctstate NEW,RELATED -j ACCE
 nft add rule ip filter INPUT ct state new,related counter accept
 
 ip6tables-translate -t filter -A INPUT -m conntrack ! --ctstate NEW,RELATED -j ACCEPT
-nft add rule ip6 filter INPUT ct state != new,related counter accept
+nft add rule ip6 filter INPUT ct state != { new,related } counter accept
+
+ip6tables-translate -t filter -A INPUT -m conntrack ! --ctstate NEW -j ACCEPT
+nft add rule ip6 filter INPUT ct state != new counter accept
 
 iptables-translate -t filter -A INPUT -m conntrack --ctproto UDP -j ACCEPT
 nft add rule ip filter INPUT ct original protocol 17 counter accept
-- 
1.8.3.1

[iptables PATCH v2 2/2] extensions: libxt_conntrack: print xlate status as set

[Alexander Mikhalitsyn]

status_xlate_print function prints statusmask
without { ... } around. But if ctstatus condition is
negative, then we have to use { ... } after "!=" operator in nft

Reproducer:
$ iptables -A INPUT -d 127.0.0.1/32 -p tcp -m conntrack ! --ctstatus expected,assured -j DROP
$ nft list ruleset
...
meta l4proto tcp ip daddr 127.0.0.1 ct status != expected,assured counter packets 0 bytes 0 drop
...

it will fail if we try to load this rule:
$ nft -f nft_test
../nft_test:6:97-97: Error: syntax error, unexpected comma, expecting newline or semicolon

Signed-off-by: Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com>
---
 extensions/libxt_conntrack.c      | 15 +++++++++++++--
 extensions/libxt_conntrack.txlate |  3 +++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/extensions/libxt_conntrack.c b/extensions/libxt_conntrack.c
index fe964aa..61a67b0 100644
--- a/extensions/libxt_conntrack.c
+++ b/extensions/libxt_conntrack.c
@@ -1198,9 +1198,16 @@ static int state_xlate(struct xt_xlate *xl,
 	return 1;
 }
 
-static void status_xlate_print(struct xt_xlate *xl, unsigned int statusmask)
+static void status_xlate_print(struct xt_xlate *xl, unsigned int statusmask, int afterinv)
 {
 	const char *sep = "";
+	int as_set;
+
+	/* print as set only after inversion and if more than one flag is set */
+	as_set = afterinv && (statusmask & (statusmask - 1));
+
+	if (as_set)
+		xt_xlate_add(xl, "{ ");
 
 	if (statusmask & IPS_EXPECTED) {
 		xt_xlate_add(xl, "%s%s", sep, "expected");
@@ -1218,6 +1225,9 @@ static void status_xlate_print(struct xt_xlate *xl, unsigned int statusmask)
 		xt_xlate_add(xl, "%s%s", sep, "confirmed");
 		sep = ",";
 	}
+
+	if (as_set)
+		xt_xlate_add(xl, " }");
 }
 
 static void addr_xlate_print(struct xt_xlate *xl,
@@ -1280,7 +1290,8 @@ static int _conntrack3_mt_xlate(struct xt_xlate *xl,
 		xt_xlate_add(xl, "%sct status %s", space,
 			     sinfo->invert_flags & XT_CONNTRACK_STATUS ?
 			     "!= " : "");
-		status_xlate_print(xl, sinfo->status_mask);
+		status_xlate_print(xl, sinfo->status_mask,
+				   sinfo->invert_flags & XT_CONNTRACK_STATUS);
 		space = " ";
 	}
 
diff --git a/extensions/libxt_conntrack.txlate b/extensions/libxt_conntrack.txlate
index 75b3daa..0cc7513 100644
--- a/extensions/libxt_conntrack.txlate
+++ b/extensions/libxt_conntrack.txlate
@@ -37,6 +37,9 @@ nft add rule ip filter INPUT ct status expected counter accept
 iptables-translate -t filter -A INPUT -m conntrack ! --ctstatus CONFIRMED -j ACCEPT
 nft add rule ip filter INPUT ct status != confirmed counter accept
 
+iptables-translate -t filter -A INPUT -m conntrack ! --ctstatus CONFIRMED,ASSURED -j ACCEPT
+nft add rule ip filter INPUT ct status != { assured,confirmed } counter accept
+
 iptables-translate -t filter -A INPUT -m conntrack --ctexpire 3 -j ACCEPT
 nft add rule ip filter INPUT ct expiration 3 counter accept
 
-- 
1.8.3.1

Re: [iptables PATCH v2 2/2] extensions: libxt_conntrack: print xlate status as set

[Florian Westphal]

Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com> wrote:
> status_xlate_print function prints statusmask
> without { ... } around. But if ctstatus condition is
> negative, then we have to use { ... } after "!=" operator in nft

Not really.

> Reproducer:
> $ iptables -A INPUT -d 127.0.0.1/32 -p tcp -m conntrack ! --ctstatus expected,assured -j DROP
> $ nft list ruleset
> ...
> meta l4proto tcp ip daddr 127.0.0.1 ct status != expected,assured counter packets 0 bytes 0 drop
> ...

Yes, nft can't parse that.

But ct status { expect, assured } is NOT The same as 'ct status expect,assured'.

expect, assured etc. are all bit flags, so when negating this needs to be something
like  'ct status & (expected|assured) == 0'. (ct is neither expected nor assured).

Re: [iptables PATCH v2 2/2] extensions: libxt_conntrack: print xlate status as set

[Alexander Mikhalitsyn]

Hi Florian,

Thank you!
So, I need to fix nft and support that syntax?

Do I understand correctly, that the same issue for state flags like
"established, related, ..."?

Regards,
Alex

________________________________________
From: Florian Westphal <fw@strlen.de>
Sent: Tuesday, March 30, 2021 20:39
To: Alexander Mikhalitsyn
Cc: netfilter-devel@vger.kernel.org; pablo@netfilter.org; fw@strlen.de
Subject: Re: [iptables PATCH v2 2/2] extensions: libxt_conntrack: print xlate status as set

Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com> wrote:
> status_xlate_print function prints statusmask
> without { ... } around. But if ctstatus condition is
> negative, then we have to use { ... } after "!=" operator in nft

Not really.

> Reproducer:
> $ iptables -A INPUT -d 127.0.0.1/32 -p tcp -m conntrack ! --ctstatus expected,assured -j DROP
> $ nft list ruleset
> ...
> meta l4proto tcp ip daddr 127.0.0.1 ct status != expected,assured counter packets 0 bytes 0 drop
> ...

Yes, nft can't parse that.

But ct status { expect, assured } is NOT The same as 'ct status expect,assured'.

expect, assured etc. are all bit flags, so when negating this needs to be something
like  'ct status & (expected|assured) == 0'. (ct is neither expected nor assured).

Re: [iptables PATCH v2 2/2] extensions: libxt_conntrack: print xlate status as set

[Florian Westphal]

Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com> wrote:
> Hi Florian,
> 
> Thank you!
> So, I need to fix nft and support that syntax?

That would be one way.  The other is to fix the != translation
to use binary logic (the example i gave).

> Do I understand correctly, that the same issue for state flags like
> "established, related, ..."?

Yes and no.  A connection can't be both established and related at the
same time, so anonymous set will work in that case.

Re: [iptables PATCH v2 2/2] extensions: libxt_conntrack: print xlate status as set

[Alexander Mikhalitsyn]

Hi, Florian,

On Tue, 30 Mar 2021 20:21:36 +0200
Florian Westphal <fw@strlen.de> wrote:

> Alexander Mikhalitsyn <alexander.mikhalitsyn@virtuozzo.com> wrote:
> > Hi Florian,
> > 
> > Thank you!
> > So, I need to fix nft and support that syntax?
> 
> That would be one way.  The other is to fix the != translation
> to use binary logic (the example i gave).

I've prepared 3rd version of patchset.

> 
> > Do I understand correctly, that the same issue for state flags like
> > "established, related, ..."?
> 
> Yes and no.  A connection can't be both established and related at the
> same time, so anonymous set will work in that case.

Got it.

Thank you very much!

Regards,
Alex