Hmm, that means `tcp flags & (fin | syn | rst | ack) syn` is now
equivalent to 'tcp flags & (fin | syn | rst | ack) == syn'. Does that
mean `tcp flags syn` (was supposed to be and) is now equivalent to
`tcp flags == syn` instead of `tcp flags & syn == syn` / `tcp flags &
syn != 0`?
Suppose `tcp flags & syn != 0` should then be translated to `tcp flags
syn / syn` instead, please note that while nft translates `tcp flags &
syn == syn` to `tcp flags syn / syn`, it does not accept the
translation as input (when the mask is not a comma-separated list):
# nft --debug=netlink add rule meh tcp_flags 'tcp flags syn / syn'
Error: syntax error, unexpected newline, expecting comma
add rule meh tcp_flags tcp flags syn / syn
^
Also, does that mean `tcp flags & (fin | syn | rst | ack) fin,syn,ack`
will now be equivalent to `tcp flags & (fin | syn | rst | ack) = (fin
| syn | ack)` instead of (ultimately) `tcp flags & (fin | syn | ack)
!= 0`? Which means `tcp flags & (fin | syn | ack) != 0` should not be
translated to `tcp flags fin,syn,ack`?
On Tue, 27 Jul 2021 at 23:37, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>
> nft generates incorrect bytecode when combining flag datatype and binary
> operations:
>
> # nft --debug=netlink add rule meh tcp_flags 'tcp flags & (fin | syn | rst | ack) syn'
> ip
> [ meta load l4proto => reg 1 ]
> [ cmp eq reg 1 0x00000006 ]
> [ payload load 1b @ transport header + 13 => reg 1 ]
> [ bitwise reg 1 = ( reg 1 & 0x00000017 ) ^ 0x00000000 ]
> [ bitwise reg 1 = ( reg 1 & 0x00000002 ) ^ 0x00000000 ]
> [ cmp neq reg 1 0x00000000 ]
>
> Note the double bitwise expression. The last two expressions are not
> correct either since it should match on the syn flag, ie. 0x2.
>
> After this patch, netlink bytecode generation looks correct:
>
> # nft --debug=netlink add rule meh tcp_flags 'tcp flags & (fin | syn | rst | ack) syn'
> ip
> [ meta load l4proto => reg 1 ]
> [ cmp eq reg 1 0x00000006 ]
> [ payload load 1b @ transport header + 13 => reg 1 ]
> [ bitwise reg 1 = ( reg 1 & 0x00000017 ) ^ 0x00000000 ]
> [ cmp eq reg 1 0x00000002 ]
>
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
> src/netlink_linearize.c | 38 ++++++++++++++++-----------
> tests/py/inet/tcp.t | 2 ++
> tests/py/inet/tcp.t.json | 52 +++++++++++++++++++++++++++++++++++++
> tests/py/inet/tcp.t.payload | 16 ++++++++++++
> 4 files changed, 93 insertions(+), 15 deletions(-)
>
> diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
> index 9ab3ec3ef2ff..eb53ccec1154 100644
> --- a/src/netlink_linearize.c
> +++ b/src/netlink_linearize.c
> @@ -481,23 +481,31 @@ static void netlink_gen_flagcmp(struct netlink_linearize_ctx *ctx,
> netlink_gen_raw_data(zero, expr->right->byteorder, len, &nld);
> netlink_gen_data(expr->right, &nld2);
>
> - nle = alloc_nft_expr("bitwise");
> - netlink_put_register(nle, NFTNL_EXPR_BITWISE_SREG, sreg);
> - netlink_put_register(nle, NFTNL_EXPR_BITWISE_DREG, sreg);
> - nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_LEN, len);
> - nftnl_expr_set(nle, NFTNL_EXPR_BITWISE_MASK, &nld2.value, nld2.len);
> - nftnl_expr_set(nle, NFTNL_EXPR_BITWISE_XOR, &nld.value, nld.len);
> - nft_rule_add_expr(ctx, nle, &expr->location);
> -
> - nle = alloc_nft_expr("cmp");
> - netlink_put_register(nle, NFTNL_EXPR_CMP_SREG, sreg);
> - if (expr->op == OP_NEG)
> + if (expr->left->etype == EXPR_BINOP) {
> + nle = alloc_nft_expr("cmp");
> + netlink_put_register(nle, NFTNL_EXPR_CMP_SREG, sreg);
> nftnl_expr_set_u32(nle, NFTNL_EXPR_CMP_OP, NFT_CMP_EQ);
> - else
> - nftnl_expr_set_u32(nle, NFTNL_EXPR_CMP_OP, NFT_CMP_NEQ);
> + nftnl_expr_set(nle, NFTNL_EXPR_CMP_DATA, nld2.value, nld2.len);
> + nft_rule_add_expr(ctx, nle, &expr->location);
> + } else {
> + nle = alloc_nft_expr("bitwise");
> + netlink_put_register(nle, NFTNL_EXPR_BITWISE_SREG, sreg);
> + netlink_put_register(nle, NFTNL_EXPR_BITWISE_DREG, sreg);
> + nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_LEN, len);
> + nftnl_expr_set(nle, NFTNL_EXPR_BITWISE_MASK, &nld2.value, nld2.len);
> + nftnl_expr_set(nle, NFTNL_EXPR_BITWISE_XOR, &nld.value, nld.len);
> + nft_rule_add_expr(ctx, nle, &expr->location);
>
> - nftnl_expr_set(nle, NFTNL_EXPR_CMP_DATA, nld.value, nld.len);
> - nft_rule_add_expr(ctx, nle, &expr->location);
> + nle = alloc_nft_expr("cmp");
> + netlink_put_register(nle, NFTNL_EXPR_CMP_SREG, sreg);
> + if (expr->op == OP_NEG)
> + nftnl_expr_set_u32(nle, NFTNL_EXPR_CMP_OP, NFT_CMP_EQ);
> + else
> + nftnl_expr_set_u32(nle, NFTNL_EXPR_CMP_OP, NFT_CMP_NEQ);
> +
> + nftnl_expr_set(nle, NFTNL_EXPR_CMP_DATA, nld.value, nld.len);
> + nft_rule_add_expr(ctx, nle, &expr->location);
> + }
>
> mpz_clear(zero);
> release_register(ctx, expr->left);
> diff --git a/tests/py/inet/tcp.t b/tests/py/inet/tcp.t
> index 16e15b9f76c1..983564ec5b75 100644
> --- a/tests/py/inet/tcp.t
> +++ b/tests/py/inet/tcp.t
> @@ -69,6 +69,8 @@ tcp flags != cwr;ok
> tcp flags == syn;ok
> tcp flags fin,syn / fin,syn;ok
> tcp flags != syn / fin,syn;ok
> +tcp flags & (fin | syn | rst | ack) syn;ok;tcp flags syn / fin,syn,rst,ack
> +tcp flags & (fin | syn | rst | ack) != syn;ok;tcp flags != syn / fin,syn,rst,ack
> tcp flags & (fin | syn | rst | psh | ack | urg | ecn | cwr) == fin | syn | rst | psh | ack | urg | ecn | cwr;ok;tcp flags == 0xff
> tcp flags { syn, syn | ack };ok
> tcp flags & (fin | syn | rst | psh | ack | urg) == { fin, ack, psh | ack, fin | psh | ack };ok
> diff --git a/tests/py/inet/tcp.t.json b/tests/py/inet/tcp.t.json
> index 590a3dee5d3f..033a4f22e0fd 100644
> --- a/tests/py/inet/tcp.t.json
> +++ b/tests/py/inet/tcp.t.json
> @@ -1521,3 +1521,55 @@
> }
> ]
>
> +# tcp flags & (fin | syn | rst | ack) syn
> +[
> + {
> + "match": {
> + "left": {
> + "&": [
> + {
> + "payload": {
> + "field": "flags",
> + "protocol": "tcp"
> + }
> + },
> + [
> + "fin",
> + "syn",
> + "rst",
> + "ack"
> + ]
> + ]
> + },
> + "op": "==",
> + "right": "syn"
> + }
> + }
> +]
> +
> +# tcp flags & (fin | syn | rst | ack) != syn
> +[
> + {
> + "match": {
> + "left": {
> + "&": [
> + {
> + "payload": {
> + "field": "flags",
> + "protocol": "tcp"
> + }
> + },
> + [
> + "fin",
> + "syn",
> + "rst",
> + "ack"
> + ]
> + ]
> + },
> + "op": "!=",
> + "right": "syn"
> + }
> + }
> +]
> +
> diff --git a/tests/py/inet/tcp.t.payload b/tests/py/inet/tcp.t.payload
> index 7f302080f02a..eaa7cd099bd6 100644
> --- a/tests/py/inet/tcp.t.payload
> +++ b/tests/py/inet/tcp.t.payload
> @@ -370,6 +370,22 @@ inet test-inet input
> [ bitwise reg 1 = ( reg 1 & 0x00000003 ) ^ 0x00000000 ]
> [ cmp neq reg 1 0x00000002 ]
>
> +# tcp flags & (fin | syn | rst | ack) syn
> +inet test-inet input
> + [ meta load l4proto => reg 1 ]
> + [ cmp eq reg 1 0x00000006 ]
> + [ payload load 1b @ transport header + 13 => reg 1 ]
> + [ bitwise reg 1 = ( reg 1 & 0x00000017 ) ^ 0x00000000 ]
> + [ cmp eq reg 1 0x00000002 ]
> +
> +# tcp flags & (fin | syn | rst | ack) != syn
> +inet test-inet input
> + [ meta load l4proto => reg 1 ]
> + [ cmp eq reg 1 0x00000006 ]
> + [ payload load 1b @ transport header + 13 => reg 1 ]
> + [ bitwise reg 1 = ( reg 1 & 0x00000017 ) ^ 0x00000000 ]
> + [ cmp neq reg 1 0x00000002 ]
> +
> # tcp flags & (fin | syn | rst | psh | ack | urg | ecn | cwr) == fin | syn | rst | psh | ack | urg | ecn | cwr
> inet test-inet input
> [ meta load l4proto => reg 1 ]
> --
> 2.20.1
>
