evaluate: place byteorder conversion before rshift in payload expressions

commit cb9b72a43c5684379c027908d9f332170bf8dd15 upstream.

Use the key from the evaluation context to perform the byteorder
conversion in case that this expression is used for lookups and updates
on explicit sets.

 # nft --debug=netlink add rule ip6 t output ip6 dscp @mapv6
 ip6 t output
  [ payload load 2b @ network header + 0 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
  [ byteorder reg 1 = ntoh(reg 1, 2, 2) ]   <-------------- this was missing!
  [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
  [ lookup reg 1 set mapv6 ]

Also with set statements (updates from packet path):

 # nft --debug=netlink add rule ip6 t output update @mapv6 { ip6 dscp }
 ip6 t output
  [ payload load 2b @ network header + 0 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
  [ byteorder reg 1 = ntoh(reg 1, 2, 2) ]   <------------- also here!
  [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
  [ dynset update reg_key 1 set mapv6 ]

Simple matches on values and implicit sets rely on the binary transfer
mechanism to propagate the shift to the constant, no explicit byteorder
is required in such case.

Fixes: 668c18f67203 ("evaluate: place byteorder conversion before rshift in payload statement")
Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/evaluate.c b/src/evaluate.c
index e063f7bade3ef718a85070f9b1f649a58d9bdf72..616fc403b62c8d4da390c4a3aef41eda1e784361 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -525,7 +525,8 @@ static void expr_evaluate_bits(struct eval_ctx *ctx, struct expr **exprp)
        and->len        = masklen;
 
        if (shift) {
-               if (ctx->stmt_len > 0 && div_round_up(masklen, BITS_PER_BYTE) > 1) {
+               if ((ctx->ectx.key || ctx->stmt_len > 0) &&
+                   div_round_up(masklen, BITS_PER_BYTE) > 1) {
                        int op = byteorder_conversion_op(expr, BYTEORDER_HOST_ENDIAN);
                        and = unary_expr_alloc(&expr->location, op, and);
                        and->len = masklen;
@@ -554,6 +555,7 @@ static void expr_evaluate_bits(struct eval_ctx *ctx, struct expr **exprp)
 
 static int __expr_evaluate_exthdr(struct eval_ctx *ctx, struct expr **exprp)
 {
+       const struct expr *key = ctx->ectx.key;
        struct expr *expr = *exprp;
 
        if (expr->exthdr.flags & NFT_EXTHDR_F_PRESENT)
@@ -562,6 +564,8 @@ static int __expr_evaluate_exthdr(struct eval_ctx *ctx, struct expr **exprp)
        if (expr_evaluate_primary(ctx, exprp) < 0)
                return -1;
 
+       ctx->ectx.key = key;
+
        if (expr->exthdr.offset % BITS_PER_BYTE != 0 ||
            expr->len % BITS_PER_BYTE != 0)
                expr_evaluate_bits(ctx, exprp);
@@ -841,6 +845,7 @@ static bool payload_needs_adjustment(const struct expr *expr)
 
 static int expr_evaluate_payload(struct eval_ctx *ctx, struct expr **exprp)
 {
+       const struct expr *key = ctx->ectx.key;
        struct expr *expr = *exprp;
 
        if (expr->payload.evaluated)
@@ -852,6 +857,8 @@ static int expr_evaluate_payload(struct eval_ctx *ctx, struct expr **exprp)
        if (expr_evaluate_primary(ctx, exprp) < 0)
                return -1;
 
+       ctx->ectx.key = key;
+
        if (payload_needs_adjustment(expr))
                expr_evaluate_bits(ctx, exprp);
 
@@ -1407,6 +1414,7 @@ static int expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr)
                }
 
                __expr_set_context(&ctx->ectx, tmp, bo, dsize, 0);
+               ctx->ectx.key = i;
 
                if (list_member_evaluate(ctx, &i) < 0)
                        return -1;