[PATCH nft 1/3] src: generate set members using integer_type in the appropriate byteorder

[PATCH nft 1/3] src: generate set members using integer_type in the appropriate byteorder

[Pablo Neira Ayuso]

Rules with header fields that rely on the generic integer datatype
from sets are not matching, eg.

 nft add rule filter input udp length { 9 } counter

This set member is an integer represented in host byte order, which
obviously doesn't match the header field (in network byte order).

Since the integer datatype has no specific byteorder, we have to rely
on the expression byteorder instead when configuring the context,
before we evaluate the list of set members.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/expression.h |   16 ++++++++++++++--
 src/evaluate.c       |    4 +++-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index 59fa5f3..4b96879 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -96,19 +96,31 @@ enum symbol_types {
  * struct expr_ctx - type context for symbol parsing during evaluation
  *
  * @dtype:	expected datatype
+ * @byteorder:	expected byteorder
  * @len:	expected len
  */
 struct expr_ctx {
 	const struct datatype	*dtype;
+	enum byteorder		byteorder;
 	unsigned int		len;
 };
 
+static inline void __expr_set_context(struct expr_ctx *ctx,
+				      const struct datatype *dtype,
+				      enum byteorder byteorder,
+				      unsigned int len)
+{
+	ctx->dtype	= dtype;
+	ctx->byteorder	= byteorder;
+	ctx->len	= len;
+}
+
 static inline void expr_set_context(struct expr_ctx *ctx,
 				    const struct datatype *dtype,
 				    unsigned int len)
 {
-	ctx->dtype = dtype;
-	ctx->len   = len;
+	__expr_set_context(ctx, dtype,
+			   dtype ? dtype->byteorder : BYTEORDER_INVALID, len);
 }
 
 /**
diff --git a/src/evaluate.c b/src/evaluate.c
index 00e55b7..0732660 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -238,6 +238,7 @@ static int expr_evaluate_value(struct eval_ctx *ctx, struct expr **expr)
 			mpz_clear(mask);
 			return -1;
 		}
+		(*expr)->byteorder = ctx->ectx.byteorder;
 		(*expr)->len = ctx->ectx.len;
 		mpz_clear(mask);
 		break;
@@ -261,7 +262,8 @@ static int expr_evaluate_value(struct eval_ctx *ctx, struct expr **expr)
  */
 static int expr_evaluate_primary(struct eval_ctx *ctx, struct expr **expr)
 {
-	expr_set_context(&ctx->ectx, (*expr)->dtype, (*expr)->len);
+	__expr_set_context(&ctx->ectx, (*expr)->dtype, (*expr)->byteorder,
+			   (*expr)->len);
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH nft 2/3] netlink_delinearize: fix listing of set members in host byteorder using integer_type

[Pablo Neira Ayuso]

 nft list table filter
 ...
        cpu { 50331648, 33554432, 0, 16777216} counter packets 8 bytes 344

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/netlink_delinearize.c |   29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index c809bb6..e9a04dd 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -742,6 +742,29 @@ static void payload_dependency_store(struct rule_pp_ctx *ctx,
 	ctx->pdep  = stmt;
 }
 
+static void integer_type_postprocess(struct expr *expr)
+{
+	struct expr *i;
+
+	switch (expr->ops->type) {
+	case EXPR_VALUE:
+		if (expr->byteorder == BYTEORDER_HOST_ENDIAN) {
+			uint32_t len = div_round_up(expr->len, BITS_PER_BYTE);
+
+			mpz_switch_byteorder(expr->value, len);
+		}
+		break;
+	case EXPR_SET_REF:
+		list_for_each_entry(i, &expr->set->init->expressions, list) {
+			expr_set_type(i, expr->dtype, expr->byteorder);
+			integer_type_postprocess(i);
+		}
+		break;
+	default:
+		break;
+	}
+}
+
 static void payload_match_postprocess(struct rule_pp_ctx *ctx,
 				      struct stmt *stmt, struct expr *expr)
 {
@@ -805,6 +828,12 @@ static void meta_match_postprocess(struct rule_pp_ctx *ctx,
 		    left->flags & EXPR_F_PROTOCOL)
 			payload_dependency_store(ctx, stmt, left->meta.base);
 		break;
+	case OP_LOOKUP:
+		expr_set_type(expr->right, expr->left->dtype,
+			      expr->left->byteorder);
+		if (expr->right->dtype == &integer_type)
+			integer_type_postprocess(expr->right);
+		break;
 	default:
 		break;
 	}
-- 
1.7.10.4

[PATCH nft 3/3] netlink: fix listing of range set elements in host byteorder

[Pablo Neira Ayuso]

We have to switch the byteorder of the element in
netlink_delinearize_setelem() for non-range values only.

This fixes the listing of:

  nft add rule filter input ct mark { 0x10-0x20 } counter

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/netlink.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/netlink.c b/src/netlink.c
index 23f38b0..e59e297 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1379,7 +1379,9 @@ static int netlink_delinearize_setelem(struct nft_set_elem *nlse,
 	expr = netlink_alloc_value(&netlink_location, &nld);
 	expr->dtype	= set->keytype;
 	expr->byteorder	= set->keytype->byteorder;
-	if (expr->byteorder == BYTEORDER_HOST_ENDIAN)
+
+	if (!(set->flags & SET_F_INTERVAL) &&
+	    expr->byteorder == BYTEORDER_HOST_ENDIAN)
 		mpz_switch_byteorder(expr->value, expr->len / BITS_PER_BYTE);
 
 	if (flags & NFT_SET_ELEM_INTERVAL_END) {
-- 
1.7.10.4

Re: [PATCH nft 2/3] netlink_delinearize: fix listing of set members in host byteorder using integer_type

[Patrick McHardy]

On 08.12, Pablo Neira Ayuso wrote:
>  nft list table filter
>  ...
>         cpu { 50331648, 33554432, 0, 16777216} counter packets 8 bytes 344

I'm certain this used to work for mark values, so I'm wondering
what broke it. We might have a more fundamental bug somewhere.

Re: [PATCH nft 2/3] netlink_delinearize: fix listing of set members in host byteorder using integer_type

[Pablo Neira Ayuso]

On Tue, Dec 09, 2014 at 07:53:21AM +0000, Patrick McHardy wrote:
> On 08.12, Pablo Neira Ayuso wrote:
> >  nft list table filter
> >  ...
> >         cpu { 50331648, 33554432, 0, 16777216} counter packets 8 bytes 344
> 
> I'm certain this used to work for mark values, so I'm wondering
> what broke it. We might have a more fundamental bug somewhere.

mark values don't use the integer_type, they use mark_type which has
an specific byteorder. So they work fine.

The problem is integer_type, it has no specific byteorder. Actually,
it is left unset so it is BYTEORDER_INVALID, which is handled when
importing/exporting as BYTEORDER_BIG_ENDIAN.

Solutions that I can see are:

1) Add specific integer_types, ie. be_integer_type (subtype of
   integer) that uses BYTEORDER_BIG_ENDIAN so the datatype tells us
   the endianness, but this doesn't seem convenient:
   http://marc.info/?l=netfilter-devel&m=141806352422510&w=2

2) From the postprocess step as you suggested. In this case, if we see
   an integer_type, we use the left-hand side expr->byteorder. Note that
   integer_type is also used in several header fields (where this
   should be a big endian integer). That case currently works because
   BYTEORDER_INVALID defaults on BYTEORDER_BIG_ENDIAN when
   importing/exporting. This new integer_postprocess() function should
   be also called. This is what this patch does.

Let me know, thanks.

Re: [PATCH nft 2/3] netlink_delinearize: fix listing of set members in host byteorder using integer_type

[Patrick McHardy]

Am 9. Dezember 2014 14:19:22 GMT+00:00, schrieb Pablo Neira Ayuso <pablo@netfilter.org>:
>On Tue, Dec 09, 2014 at 07:53:21AM +0000, Patrick McHardy wrote:
>> On 08.12, Pablo Neira Ayuso wrote:
>> >  nft list table filter
>> >  ...
>> >         cpu { 50331648, 33554432, 0, 16777216} counter packets 8
>bytes 344
>> 
>> I'm certain this used to work for mark values, so I'm wondering
>> what broke it. We might have a more fundamental bug somewhere.
>
>mark values don't use the integer_type, they use mark_type which has
>an specific byteorder. So they work fine.

Right, that explains it.

>The problem is integer_type, it has no specific byteorder. Actually,
>it is left unset so it is BYTEORDER_INVALID, which is handled when
>importing/exporting as BYTEORDER_BIG_ENDIAN.
>
>Solutions that I can see are:
>
>1) Add specific integer_types, ie. be_integer_type (subtype of
>   integer) that uses BYTEORDER_BIG_ENDIAN so the datatype tells us
>   the endianness, but this doesn't seem convenient:
>   http://marc.info/?l=netfilter-devel&m=141806352422510&w=2
>
>2) From the postprocess step as you suggested. In this case, if we see
>  an integer_type, we use the left-hand side expr->byteorder. Note that
>   integer_type is also used in several header fields (where this
>   should be a big endian integer). That case currently works because
>   BYTEORDER_INVALID defaults on BYTEORDER_BIG_ENDIAN when
>   importing/exporting. This new integer_postprocess() function should
>   be also called. This is what this patch does.
>
>Let me know, thanks.

Your patch is fine, I was just wondering Id wie broke something.