[nft PATCH 0/3] fix a delinearization issue

[nft PATCH 0/3] fix a delinearization issue

[Eric Leblond]

Hello,

This small patchset is fixing a ntables delinearization issue when
using counter. The rule triggering this is not straightforward as
it relays on statement order:
 ip protocol tcp counter packets tcp dport ssh accept
But it is possible some users are using this kind of rules so it
should be linearized and delinearized correctly.

With current code it was converted when reading rules from kernel
to:
 counter packets tcp dport ssh accept

BR,
--
Eric

[nft PATCH 1/3] regression: fix typo in README

[Eric Leblond]

Signed-off-by: Eric Leblond <eric@regit.org>
---
 tests/regression/README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/regression/README b/tests/regression/README
index 7b66d22..82d73a2 100644
--- a/tests/regression/README
+++ b/tests/regression/README
@@ -112,7 +112,7 @@ bridge and any.
  * "ip" folder contains the test files that are executed in ip and inet
    table.
 
- * "ip" folder contains the test files that are executed in ip6 and inet
+ * "ip6" folder contains the test files that are executed in ip6 and inet
    table.
 
  * "inet" folder contains the test files that are executed in the ip, ip6
-- 
2.1.4

[nft PATCH 2/3] regression: add test on counter related issue

[Eric Leblond]

Signed-off-by: Eric Leblond <eric@regit.org>
---
 tests/regression/ip/ip.t | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/regression/ip/ip.t b/tests/regression/ip/ip.t
index a781de5..0d056ab 100644
--- a/tests/regression/ip/ip.t
+++ b/tests/regression/ip/ip.t
@@ -105,3 +105,5 @@ ip saddr != 192.168.1.3-192.168.33.55;ok;ip saddr < 192.168.1.3 ip saddr > 192.1
 ip daddr 192.168.0.1;ok
 ip daddr 192.168.0.1 drop;ok
 ip daddr 192.168.0.2 log;ok
+
+ip protocol 6 counter tcp dport 223 accept;ok;ip protocol 6 counter packets 0 bytes 0 tcp dport 223 accept
-- 
2.1.4

[nft PATCH 3/3] netlink_delinearize: fix postprocessing

[Eric Leblond]

The following rule:
  ip protocol tcp counter packets 0 bytes 0 tcp dport ssh accept

is build in byte code as:

  ip test filter
    [ payload load 1b @ network header + 9 => reg 1 ]
    [ cmp eq reg 1 0x00000006 ]
    [ counter pkts 0 bytes 0 ]
    [ payload load 2b @ transport header + 2 => reg 1 ]
    [ cmp eq reg 1 0x00001600 ]
    [ immediate reg 0 accept ]

But the simplication process is reverting it to:
  counter tcp dport ssh accept

Which is different rule.

This patch is fixing the issue by resetting the dependency when we
are seeing a counter statement.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 src/netlink_delinearize.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 387bb67..181942b 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -770,6 +770,12 @@ static void payload_dependency_store(struct rule_pp_ctx *ctx,
 	ctx->pdep  = stmt;
 }
 
+static void payload_dependency_reset(struct rule_pp_ctx *ctx)
+{
+	ctx->pbase = PROTO_BASE_INVALID;
+	ctx->pdep  = NULL;
+}
+
 static void integer_type_postprocess(struct expr *expr)
 {
 	struct expr *i;
@@ -1137,6 +1143,9 @@ static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *r
 		case STMT_REJECT:
 			stmt_reject_postprocess(rctx, stmt);
 			break;
+		case STMT_COUNTER:
+			payload_dependency_reset(&rctx);
+			break;
 		default:
 			break;
 		}
-- 
2.1.4

Re: [nft PATCH 3/3] netlink_delinearize: fix postprocessing

[Patrick McHardy]

On 26.02, Eric Leblond wrote:
> The following rule:
>   ip protocol tcp counter packets 0 bytes 0 tcp dport ssh accept
> 
> is build in byte code as:
> 
>   ip test filter
>     [ payload load 1b @ network header + 9 => reg 1 ]
>     [ cmp eq reg 1 0x00000006 ]
>     [ counter pkts 0 bytes 0 ]
>     [ payload load 2b @ transport header + 2 => reg 1 ]
>     [ cmp eq reg 1 0x00001600 ]
>     [ immediate reg 0 accept ]
> 
> But the simplication process is reverting it to:
>   counter tcp dport ssh accept
> 
> Which is different rule.
> 
> This patch is fixing the issue by resetting the dependency when we
> are seeing a counter statement.

This isn't specific to the counter statement but any kind of statement
that is non-terminal.

Or more generally speaking, we only want to undo the automatic payload
expression generation, which always generates expressions like this

1. dependency expression (generated)
2. dependant expression

So I'd argue that we should at least reset it for all statements other
than STMT_EXPR. That case is more complicated to handle and I'd leave it
for now.

> @@ -1137,6 +1143,9 @@ static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *r
>  		case STMT_REJECT:
>  			stmt_reject_postprocess(rctx, stmt);
>  			break;
> +		case STMT_COUNTER:
> +			payload_dependency_reset(&rctx);
> +			break;
>  		default:
>  			break;
>  		}
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>