payload: don't adjust offsets of autogenerated dependency expressions

Pablo says:
  user reports that this is broken:
  nft --debug=netlink add rule bridge filter forward vlan id 100 vlan id set 200
[..]
    [ payload load 2b @ link header + 14 => reg 1 ]
[..]
    [ payload load 2b @ link header + 28 => reg 1 ]
    [ bitwise reg 1 = ( reg 1 & 0x000000f0 ) ^ 0x0000c800 ]
    [ payload write reg 1 => 2b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ]

    offset says 28, it is assuming q-in-q, in this case it is mangling the
    existing header.

The problem here is that 'vlan id set 200' needs a read-modify-write
cycle because 'vlan id set' has to preserve bits located in the same byte area
as the vlan id.

The first 'payload load' at offset 14 is generated via 'vlan id 100',
this part is ok.

The second 'payload load' at offset 28 is the bogus one.
Its added as a dependency, but then adjusted because nft evaluation
considers this identical to 'vlan id 1 vlan id '2, where nft assumes
q-in-q.

To fix this, skip offset adjustments for raw expressions and mark the
dependency-generated payload instruction as such.

This is fine because raw payload operations assume that user specifies
base/offset/length manually.

Also add a test case for this.

Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>

diff --git a/src/evaluate.c b/src/evaluate.c
index 8ebc75617b1c5fe1de1595f3737925bc9e30499c..b39f45981c422e14bcb12214444293eafa05721a 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2554,6 +2554,7 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
                         payload_byte_offset * BITS_PER_BYTE,
                         payload_byte_size * BITS_PER_BYTE);
 
+       payload_bytes->payload.is_raw = 1;
        payload_bytes->payload.desc      = payload->payload.desc;
        payload_bytes->byteorder         = payload->byteorder;
 

diff --git a/src/payload.c b/src/payload.c
index 97b60713e8008249488121fdca66ea28f1f7d903..c662900bdaac31a6dc3985a41f4fcadf97d48494 100644 (file)
--- a/src/payload.c
+++ b/src/payload.c
@@ -115,7 +115,9 @@ static void payload_expr_pctx_update(struct proto_ctx *ctx,
        assert(desc->base <= PROTO_BASE_MAX);
        if (desc->base == base->base) {
                assert(base->length > 0);
-               ctx->protocol[base->base].offset += base->length;
+
+               if (!left->payload.is_raw)
+                       ctx->protocol[base->base].offset += base->length;
        }
        proto_ctx_update(ctx, desc->base, loc, desc);
 }

diff --git a/tests/py/bridge/vlan.t b/tests/py/bridge/vlan.t
index fd39d2227676644961e644cd16d5f3d677dd61a6..b506ee8df6bddea9d5b92b8225f0c13e688dbf18 100644 (file)
--- a/tests/py/bridge/vlan.t
+++ b/tests/py/bridge/vlan.t
@@ -43,3 +43,6 @@ ether type 8021ad vlan id 1 vlan type 8021q vlan id 2 vlan type ip ip protocol 6
 # illegal dependencies
 ether type ip vlan id 1;fail
 ether type ip vlan id 1 ip saddr 10.0.0.1;fail
+
+# mangling
+vlan id 1 vlan id set 2;ok

diff --git a/tests/py/bridge/vlan.t.json b/tests/py/bridge/vlan.t.json
index dae70170398a2ba239bd80aff55987ee5d704551..e7640f9a6a379d53ed3748fc11b65c4afea22526 100644 (file)
--- a/tests/py/bridge/vlan.t.json
+++ b/tests/py/bridge/vlan.t.json
@@ -734,3 +734,30 @@
         }
     }
 ]
+
+# vlan id 1 vlan id set 2
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "vlan"
+                }
+            },
+            "op": "==",
+            "right": 1
+        }
+    },
+    {
+        "mangle": {
+            "key": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "vlan"
+                }
+            },
+            "value": 2
+        }
+    }
+]

diff --git a/tests/py/bridge/vlan.t.payload b/tests/py/bridge/vlan.t.payload
index 49fd0ea7ab3ba19dc4ee204427f5939d422c87c3..6c8d595a1aad0b021d31c9d78d26aeb7070bb76a 100644 (file)
--- a/tests/py/bridge/vlan.t.payload
+++ b/tests/py/bridge/vlan.t.payload
@@ -265,3 +265,14 @@ bridge
   [ cmp eq reg 1 0x00000008 ]
   [ payload load 1b @ network header + 9 => reg 1 ]
   [ cmp eq reg 1 0x00000006 ]
+
+# vlan id 1 vlan id set 2
+bridge
+  [ payload load 2b @ link header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x00000081 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000100 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000000f0 ) ^ 0x00000200 ]
+  [ payload write reg 1 => 2b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ]

diff --git a/tests/py/bridge/vlan.t.payload.netdev b/tests/py/bridge/vlan.t.payload.netdev
index 1a2c08ae7a949b1bbc9baf598c04d3a0f50f1de2..d2c7d74a4e858748921d83353b784e12bd39de8d 100644 (file)
--- a/tests/py/bridge/vlan.t.payload.netdev
+++ b/tests/py/bridge/vlan.t.payload.netdev
@@ -309,3 +309,16 @@ netdev
   [ cmp eq reg 1 0x00000008 ]
   [ payload load 1b @ network header + 9 => reg 1 ]
   [ cmp eq reg 1 0x00000006 ]
+
+# vlan id 1 vlan id set 2
+netdev
+  [ meta load iiftype => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ link header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x00000081 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000100 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000000f0 ) ^ 0x00000200 ]
+  [ payload write reg 1 => 2b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ]